[v2,110/148] net/ice/base: squash multiple fixes for e56g device

Message ID 2c087fbcc3fcd93185301d63e31c70cf639c29a1.1718204529.git.anatoly.burakov@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Bruce Richardson
Headers
Series Update net/ice base driver to latest upstream snapshot |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Burakov, Anatoly June 12, 2024, 3:01 p.m. UTC
From: Ian Stokes <ian.stokes@intel.com>

This commit has multiple fixes related to comments updates, possible unitialized
variable use and finally a bug fix related to stale PHY commands. Details of the
bug fix are included below as it represents the bulk of the changes.

Code using ice_ptp_one_port_cmd() (or the device specific variants for E822 and
ETH56G) has a subtle bug where-in the executing of ice_ptp_exec_tmr_cmd() will
cause all other ports to execute their last executed command.Code using
ice_ptp_one_port_cmd() (or the device specific variants for E822 and ETH56G) has
a subtle bug where-in the executing of ice_ptp_exec_tmr_cmd() will cause all
other ports to execute their last executed command.

Most flows operate on all timers at once, but some flows such as handling link
state change only operate on one port at a time. This can lead to issues if the
userspace performs clock operations at just the right time.

In addition to this bug, recent support for new hardware families has
complicated the structure and organization of the various functions involved in
programming PHY ports.

Refactor ice_ptp_one_port_cmd() to both fix the issue of stale PHY port
commands, and reduce the organizational complexity.

Rename the existing device implementations from ice_ptp_one_port_cmd_<device> to
ice_ptp_write_port_cmd_<device>. This function just performs the task of writing
one command to one port. Do not export this outside of ice_ptp_hw.c, instead
ensure all callers use the refactored ice_ptp_one_port_cmd() which properly
initializes all ports.

Fix ice_ptp_one_port_cmd() to correctly loop over all ports. For the configured
port, call ice_ptp_write_port_cmd() with the configured command. For all other
ports, call ice_ptp_write_port_cmd() with ICE_PTP_NOP.

Stop duplicating the port iteration logic in each of the
ice_ptp_port_cmd_<dev>() implementations, instead moving this into the generic
ice_ptp_port_cmd() implementation.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 drivers/net/ice/base/ice_ptp_hw.c | 138 +++++++-----------------------
 drivers/net/ice/base/ice_ptp_hw.h |   6 --
 drivers/net/ice/base/ice_switch.c |   3 +
 3 files changed, 33 insertions(+), 114 deletions(-)
  

Patch

diff --git a/drivers/net/ice/base/ice_ptp_hw.c b/drivers/net/ice/base/ice_ptp_hw.c
index 64c48f5f2a..54eb2953be 100644
--- a/drivers/net/ice/base/ice_ptp_hw.c
+++ b/drivers/net/ice/base/ice_ptp_hw.c
@@ -982,13 +982,15 @@  ice_read_phy_eth56g_raw_lp(struct ice_hw *hw, u8 phy_index, u32 reg_addr,
 
 	err = ice_sbq_rw_reg_lp(hw, &phy_msg, lock_sbq);
 
-	if (err)
+	if (err) {
 		ice_debug(hw, ICE_DBG_PTP, "PTP failed to send msg to phy %d\n",
 			  err);
-	else
-		*val = phy_msg.data;
+		return err;
+	}
 
-	return err;
+	*val = phy_msg.data;
+
+	return ICE_SUCCESS;
 }
 
 /**
@@ -1479,8 +1481,16 @@  ice_read_phy_tstamp_eth56g(struct ice_hw *hw, u8 port, u8 idx, u64 *tstamp)
  * @port: the quad to read from
  * @idx: the timestamp index to reset
  *
- * Clear a timestamp, resetting its valid bit, in the PHY port memory of
+ * Read and then forcibly clear the timestamp index to ensure the valid bit is
+ * cleared and the timestamp status bit is reset in the PHY port memory of
  * 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().
+ *
+ * This function should only be called on an idx whose bit is set according to
+ * ice_get_phy_tx_tstamp_ready().
  */
 static int
 ice_clear_phy_tstamp_eth56g(struct ice_hw *hw, u8 port, u8 idx)
@@ -1509,9 +1519,6 @@  static void ice_ptp_reset_ts_memory_eth56g(struct ice_hw *hw)
 	unsigned int port;
 
 	for (port = 0; port < hw->max_phy_port; port++) {
-		if (!(hw->ena_lports & BIT(port)))
-			continue;
-
 		ice_write_phy_reg_eth56g(hw, port, PHY_REG_TX_MEMORY_STATUS_L,
 					 0);
 		ice_write_phy_reg_eth56g(hw, port, PHY_REG_TX_MEMORY_STATUS_U,
@@ -1568,8 +1575,7 @@  ice_ptp_prep_phy_time_eth56g(struct ice_hw *hw, u32 time)
 
 	for (port = 0; port < hw->max_phy_port; port++) {
 		int err;
-		if (!(hw->ena_lports & BIT(port)))
-			continue;
+
 		err = ice_ptp_prep_port_phy_time_eth56g(hw, port, phy_time);
 
 		if (err) {
@@ -1664,8 +1670,6 @@  ice_ptp_prep_phy_adj_eth56g(struct ice_hw *hw, s32 adj, bool lock_sbq)
 	cycles = (s64)adj << 32;
 
 	for (port = 0; port < hw->max_phy_port; port++) {
-		if (!(hw->ena_lports & BIT(port)))
-			continue;
 
 		err = ice_ptp_prep_port_adj_eth56g(hw, port, cycles, lock_sbq);
 		if (err)
@@ -1691,8 +1695,6 @@  ice_ptp_prep_phy_incval_eth56g(struct ice_hw *hw, u64 incval)
 
 	for (port = 0; port < hw->max_phy_port; port++) {
 		int err;
-		if (!(hw->ena_lports & BIT(port)))
-			continue;
 		err = ice_write_40b_phy_reg_eth56g(hw, port, PHY_REG_TIMETUS_L,
 						   incval);
 		if (err) {
@@ -1752,9 +1754,6 @@  ice_ptp_prep_phy_adj_target_eth56g(struct ice_hw *hw, u32 target_time)
 	u8 port;
 
 	for (port = 0; port < hw->max_phy_port; port++) {
-		if (!(hw->ena_lports & BIT(port)))
-			continue;
-
 		/* Tx case */
 		/* No sub-nanoseconds data */
 		err = ice_write_phy_reg_eth56g_lp(hw, port,
@@ -1836,7 +1835,7 @@  ice_ptp_read_port_capture_eth56g(struct ice_hw *hw, u8 port, u64 *tx_ts,
 }
 
 /**
- * ice_ptp_one_port_cmd_eth56g - Prepare a single PHY port for a timer command
+ * ice_ptp_write_port_cmd_eth56g - Prepare a single PHY port for a timer command
  * @hw: pointer to HW struct
  * @port: Port to which cmd has to be sent
  * @cmd: Command to be sent to the port
@@ -1844,53 +1843,16 @@  ice_ptp_read_port_capture_eth56g(struct ice_hw *hw, u8 port, u64 *tx_ts,
  *
  * Prepare the requested port for an upcoming timer sync command.
  */
-int
-ice_ptp_one_port_cmd_eth56g(struct ice_hw *hw, u8 port,
-			    enum ice_ptp_tmr_cmd cmd, bool lock_sbq)
+static int
+ice_ptp_write_port_cmd_eth56g(struct ice_hw *hw, u8 port,
+			      enum ice_ptp_tmr_cmd cmd, bool lock_sbq)
 {
+	u32 val = ice_ptp_tmr_cmd_to_port_reg(hw, cmd);
 	int err;
-	u32 cmd_val, val;
-	u8 tmr_idx;
-
-	tmr_idx = ice_get_ptp_src_clock_index(hw);
-	cmd_val = tmr_idx << SEL_PHY_SRC;
-	switch (cmd) {
-	case ICE_PTP_INIT_TIME:
-		cmd_val |= PHY_CMD_INIT_TIME;
-		break;
-	case ICE_PTP_INIT_INCVAL:
-		cmd_val |= PHY_CMD_INIT_INCVAL;
-		break;
-	case ICE_PTP_ADJ_TIME:
-		cmd_val |= PHY_CMD_ADJ_TIME;
-		break;
-	case ICE_PTP_ADJ_TIME_AT_TIME:
-		cmd_val |= PHY_CMD_ADJ_TIME_AT_TIME;
-		break;
-	case ICE_PTP_READ_TIME:
-		cmd_val |= PHY_CMD_READ_TIME;
-		break;
-	default:
-		ice_warn(hw, "Unknown timer command %u\n", cmd);
-		return ICE_ERR_PARAM;
-	}
 
 	/* Tx case */
-	/* Read, modify, write */
-	err = ice_read_phy_reg_eth56g_lp(hw, port, PHY_REG_TX_TMR_CMD, &val,
-					    lock_sbq);
-	if (err) {
-		ice_debug(hw, ICE_DBG_PTP, "Failed to read TX_TMR_CMD, err %d\n",
-			  err);
-		return err;
-	}
-
-	/* Modify necessary bits only and perform write */
-	val &= ~TS_CMD_MASK;
-	val |= cmd_val;
-
 	err = ice_write_phy_reg_eth56g_lp(hw, port, PHY_REG_TX_TMR_CMD, val,
-					     lock_sbq);
+					  lock_sbq);
 	if (err) {
 		ice_debug(hw, ICE_DBG_PTP, "Failed to write back TX_TMR_CMD, err %d\n",
 			  err);
@@ -1898,21 +1860,8 @@  ice_ptp_one_port_cmd_eth56g(struct ice_hw *hw, u8 port,
 	}
 
 	/* Rx case */
-	/* Read, modify, write */
-	err = ice_read_phy_reg_eth56g_lp(hw, port, PHY_REG_RX_TMR_CMD, &val,
-					    lock_sbq);
-	if (err) {
-		ice_debug(hw, ICE_DBG_PTP, "Failed to read RX_TMR_CMD, err %d\n",
-			  err);
-		return err;
-	}
-
-	/* Modify necessary bits only and perform write */
-	val &= ~TS_CMD_MASK;
-	val |= cmd_val;
-
 	err = ice_write_phy_reg_eth56g_lp(hw, port, PHY_REG_RX_TMR_CMD, val,
-					     lock_sbq);
+					  lock_sbq);
 	if (err) {
 		ice_debug(hw, ICE_DBG_PTP, "Failed to write back RX_TMR_CMD, err %d\n",
 			  err);
@@ -1922,34 +1871,6 @@  ice_ptp_one_port_cmd_eth56g(struct ice_hw *hw, u8 port,
 	return 0;
 }
 
-/**
- * ice_ptp_port_cmd_eth56g - Prepare all ports for a timer command
- * @hw: pointer to the HW struct
- * @cmd: timer command to prepare
- * @lock_sbq: true if the sideband queue lock must  be acquired
- *
- * Prepare all ports connected to this device for an upcoming timer sync
- * command.
- */
-int
-ice_ptp_port_cmd_eth56g(struct ice_hw *hw, enum ice_ptp_tmr_cmd cmd,
-			bool lock_sbq)
-{
-	int err;
-	u8 port;
-
-	for (port = 0; port < hw->max_phy_port; port++) {
-		if (!(hw->ena_lports & BIT(port)))
-			continue;
-
-		err = ice_ptp_one_port_cmd_eth56g(hw, port, cmd, lock_sbq);
-		if (err)
-			return err;
-	}
-
-	return 0;
-}
-
 /**
  * ice_calc_fixed_tx_offset_eth56g - Calculated Fixed Tx offset for a port
  * @hw: pointer to the HW struct
@@ -2108,7 +2029,7 @@  ice_read_phy_and_phc_time_eth56g(struct ice_hw *hw, u8 port, u64 *phy_time,
 	ice_ptp_src_cmd(hw, ICE_PTP_READ_TIME);
 
 	/* Prepare the PHY timer for a ICE_PTP_READ_TIME capture command */
-	err = ice_ptp_one_port_cmd_eth56g(hw, port, ICE_PTP_READ_TIME, true);
+	err = ice_ptp_one_port_cmd(hw, port, ICE_PTP_READ_TIME, true);
 	if (err)
 		return err;
 
@@ -2182,7 +2103,7 @@  static int ice_sync_phy_timer_eth56g(struct ice_hw *hw, u8 port)
 	if (err)
 		goto err_unlock;
 
-	err = ice_ptp_one_port_cmd_eth56g(hw, port, ICE_PTP_ADJ_TIME, true);
+	err = ice_ptp_one_port_cmd(hw, port, ICE_PTP_ADJ_TIME, true);
 	if (err)
 		goto err_unlock;
 
@@ -2271,8 +2192,7 @@  ice_start_phy_timer_eth56g(struct ice_hw *hw, u8 port, bool bypass)
 	if (err)
 		return err;
 
-	err = ice_ptp_one_port_cmd_eth56g(hw, port, ICE_PTP_INIT_INCVAL,
-					     true);
+	err = ice_ptp_one_port_cmd(hw, port, ICE_PTP_INIT_INCVAL, true);
 	if (err)
 		return err;
 
@@ -2925,7 +2845,7 @@  ice_read_quad_reg_e822_lp(struct ice_hw *hw, u8 quad, u16 offset, u32 *val,
 
 	*val = msg.data;
 
-	return 0;
+	return ICE_SUCCESS;
 }
 
 int
@@ -2966,7 +2886,7 @@  ice_write_quad_reg_e822_lp(struct ice_hw *hw, u8 quad, u16 offset, u32 val,
 		return err;
 	}
 
-	return 0;
+	return ICE_SUCCESS;
 }
 
 int
@@ -5738,6 +5658,8 @@  int ice_ptp_write_port_cmd(struct ice_hw *hw, u8 port,
 			   enum ice_ptp_tmr_cmd cmd, bool lock_sbq)
 {
 	switch (hw->phy_model) {
+	case ICE_PHY_ETH56G:
+		return ice_ptp_write_port_cmd_eth56g(hw, port, cmd, lock_sbq);
 	case ICE_PHY_E822:
 		return ice_ptp_write_port_cmd_e822(hw, port, cmd, lock_sbq);
 	default:
diff --git a/drivers/net/ice/base/ice_ptp_hw.h b/drivers/net/ice/base/ice_ptp_hw.h
index 8b102dc0e3..b0f0a9e6c6 100644
--- a/drivers/net/ice/base/ice_ptp_hw.h
+++ b/drivers/net/ice/base/ice_ptp_hw.h
@@ -217,9 +217,6 @@  ice_ptp_port_cmd_e810(struct ice_hw *hw, enum ice_ptp_tmr_cmd cmd,
 		      bool lock_sbq);
 int
 ice_get_phy_tx_tstamp_ready_e830(struct ice_hw *hw, u8 port, u64 *tstamp_ready);
-int
-ice_ptp_port_cmd_eth56g(struct ice_hw *hw, enum ice_ptp_tmr_cmd cmd,
-			bool lock_sbq);
 
 /**
  * ice_e822_time_ref - Get the current TIME_REF from capabilities
@@ -311,9 +308,6 @@  int
 ice_ptp_read_port_capture_eth56g(struct ice_hw *hw, u8 port,
 				 u64 *tx_ts, u64 *rx_ts);
 int
-ice_ptp_one_port_cmd_eth56g(struct ice_hw *hw, u8 port,
-			    enum ice_ptp_tmr_cmd cmd, bool lock_sbq);
-int
 ice_ptp_read_tx_hwtstamp_status_eth56g(struct ice_hw *hw, u32 *ts_status);
 int
 ice_stop_phy_timer_eth56g(struct ice_hw *hw, u8 port, bool soft_reset);
diff --git a/drivers/net/ice/base/ice_switch.c b/drivers/net/ice/base/ice_switch.c
index 37c61ed480..b69c7e8b11 100644
--- a/drivers/net/ice/base/ice_switch.c
+++ b/drivers/net/ice/base/ice_switch.c
@@ -6475,6 +6475,9 @@  int
 ice_get_vsi_vlan_promisc(struct ice_hw *hw, u16 vsi_handle,
 			 ice_bitmap_t *promisc_mask, u16 *vid)
 {
+	if (!hw || !promisc_mask || !vid)
+		return ICE_ERR_PARAM;
+
 	return _ice_get_vsi_promisc(hw, vsi_handle, promisc_mask,
 				    vid, hw->switch_info,
 				    ICE_SW_LKUP_PROMISC_VLAN);