[Intel-wired-lan] [PATCH v2 iwl-net 0/4] Fix E825 initialization

[Intel-wired-lan] [PATCH v2 iwl-net 0/4] Fix E825 initialization

[Karol Kolacinski]

E825 products have incorrect initialization procedure, which may lead to
initialization failures and register values.

Fix E825 products initialization by adding correct sync delay, checking
the PHY revision only for current PHY and adding proper destination
device when reading port/quad.

In addition, E825 uses PF ID for indexing per PF registers and as
a primary PHY lane number, which is incorrect.

Karol Kolacinski (4):
  ice: Fix E825 initialization
  ice: Fix quad registers read on E825
  ice: Fix ETH56G FC-FEC Rx offset value
  ice: Add correct PHY lane assignment

 drivers/net/ethernet/intel/ice/ice_common.c   |  42 ++-
 drivers/net/ethernet/intel/ice/ice_common.h   |   1 +
 drivers/net/ethernet/intel/ice/ice_main.c     |   6 +-
 drivers/net/ethernet/intel/ice/ice_ptp.c      |  23 +-
 drivers/net/ethernet/intel/ice/ice_ptp.h      |   4 +-
 .../net/ethernet/intel/ice/ice_ptp_consts.h   |  77 +----
 drivers/net/ethernet/intel/ice/ice_ptp_hw.c   | 281 ++++++++++--------
 drivers/net/ethernet/intel/ice/ice_ptp_hw.h   |  40 ++-
 drivers/net/ethernet/intel/ice/ice_sbq_cmd.h  |   7 +-
 drivers/net/ethernet/intel/ice/ice_type.h     |   2 -
 10 files changed, 243 insertions(+), 240 deletions(-)

V1 -> V2: Removed net-next hunks from "ice: Fix E825 initialization",
          whole "ice: Remove unnecessary offset calculation for PF
          scoped registers" patch and fixed kdoc in "ice: Fix quad
          registers read on E825"

base-commit: ab8024f0207eec0f376591c958b5bd30a03ae370
-- 
2.46.2

[Intel-wired-lan] [PATCH v2 iwl-net 1/4] ice: Fix E825 initialization

[Karol Kolacinski]

Current implementation checks revision of all PHYs on all PFs, which is
incorrect and may result in initialization failure. Check only the
revision of the current PHY.

Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
---
V1 -> V2: Removed net-next hunks

 drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 22 ++++++++-------------
 drivers/net/ethernet/intel/ice/ice_ptp_hw.h |  3 ++-
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index 3a33e6b9b313..d9a3c7de0342 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -2665,14 +2665,15 @@ static bool ice_is_muxed_topo(struct ice_hw *hw)
 }
 
 /**
- * ice_ptp_init_phy_e825c - initialize PHY parameters
+ * ice_ptp_init_phy_e825 - initialize PHY parameters
  * @hw: pointer to the HW struct
  */
-static void ice_ptp_init_phy_e825c(struct ice_hw *hw)
+static void ice_ptp_init_phy_e825(struct ice_hw *hw)
 {
 	struct ice_ptp_hw *ptp = &hw->ptp;
 	struct ice_eth56g_params *params;
-	u8 phy;
+	u32 phy_rev;
+	int err;
 
 	ptp->phy_model = ICE_PHY_ETH56G;
 	params = &ptp->phy.eth56g;
@@ -2686,16 +2687,9 @@ static void ice_ptp_init_phy_e825c(struct ice_hw *hw)
 	ptp->num_lports = params->num_phys * ptp->ports_per_phy;
 
 	ice_sb_access_ena_eth56g(hw, true);
-	for (phy = 0; phy < params->num_phys; phy++) {
-		u32 phy_rev;
-		int err;
-
-		err = ice_read_phy_eth56g(hw, phy, PHY_REG_REVISION, &phy_rev);
-		if (err || phy_rev != PHY_REVISION_ETH56G) {
-			ptp->phy_model = ICE_PHY_UNSUP;
-			return;
-		}
-	}
+	err = ice_read_phy_eth56g(hw, hw->pf_id, PHY_REG_REVISION, &phy_rev);
+	if (err || phy_rev != PHY_REVISION_ETH56G)
+		ptp->phy_model = ICE_PHY_UNSUP;
 
 	ptp->is_2x50g_muxed_topo = ice_is_muxed_topo(hw);
 }
@@ -5396,7 +5390,7 @@ void ice_ptp_init_hw(struct ice_hw *hw)
 	else if (ice_is_e810(hw))
 		ice_ptp_init_phy_e810(ptp);
 	else if (ice_is_e825c(hw))
-		ice_ptp_init_phy_e825c(hw);
+		ice_ptp_init_phy_e825(hw);
 	else
 		ptp->phy_model = ICE_PHY_UNSUP;
 }
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
index 0852a34ade91..35141198f261 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
@@ -326,7 +326,8 @@ extern const struct ice_vernier_info_e82x e822_vernier[NUM_ICE_PTP_LNK_SPD];
  */
 #define ICE_E810_PLL_FREQ		812500000
 #define ICE_PTP_NOMINAL_INCVAL_E810	0x13b13b13bULL
-#define E810_OUT_PROP_DELAY_NS 1
+#define ICE_E810_OUT_PROP_DELAY_NS	1
+#define ICE_E825_SYNC_DELAY		6
 
 /* Device agnostic functions */
 u8 ice_get_ptp_src_clock_index(struct ice_hw *hw);
-- 
2.46.2

Re: [Intel-wired-lan] [PATCH v2 iwl-net 1/4] ice: Fix E825 initialization

[Jacob Keller]

On 10/10/2024 7:21 AM, Karol Kolacinski wrote:
> @@ -2686,16 +2687,9 @@ static void ice_ptp_init_phy_e825c(struct ice_hw *hw)
>  	ptp->num_lports = params->num_phys * ptp->ports_per_phy;
>  
>  	ice_sb_access_ena_eth56g(hw, true);
> -	for (phy = 0; phy < params->num_phys; phy++) {
> -		u32 phy_rev;
> -		int err;
> -
> -		err = ice_read_phy_eth56g(hw, phy, PHY_REG_REVISION, &phy_rev);
> -		if (err || phy_rev != PHY_REVISION_ETH56G) {
> -			ptp->phy_model = ICE_PHY_UNSUP;
> -			return;
> -		}
> -	}
> +	err = ice_read_phy_eth56g(hw, hw->pf_id, PHY_REG_REVISION, &phy_rev);
> +	if (err || phy_rev != PHY_REVISION_ETH56G)
> +		ptp->phy_model = ICE_PHY_UNSUP;
>  

Shouldn't this return like it did above?

>  	ptp->is_2x50g_muxed_topo = ice_is_muxed_topo(hw);
>  }

> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> index 0852a34ade91..35141198f261 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> @@ -326,7 +326,8 @@ extern const struct ice_vernier_info_e82x e822_vernier[NUM_ICE_PTP_LNK_SPD];
>   */
>  #define ICE_E810_PLL_FREQ		812500000
>  #define ICE_PTP_NOMINAL_INCVAL_E810	0x13b13b13bULL
> -#define E810_OUT_PROP_DELAY_NS 1
> +#define ICE_E810_OUT_PROP_DELAY_NS	1
> +#define ICE_E825_SYNC_DELAY		6
>  

Why is this hunk in this patch?

Re: [Intel-wired-lan] [PATCH v2 iwl-net 1/4] ice: Fix E825 initialization

[Nitka, Grzegorz]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: Thursday, October 17, 2024 1:25 AM
> To: Kolacinski, Karol <karol.kolacinski@intel.com>; intel-wired-
> lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Kubalewski, Arkadiusz
> <arkadiusz.kubalewski@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH v2 iwl-net 1/4] ice: Fix E825 initialization
> 
> 
> 
> On 10/10/2024 7:21 AM, Karol Kolacinski wrote:
> > @@ -2686,16 +2687,9 @@ static void ice_ptp_init_phy_e825c(struct
> ice_hw *hw)
> >  	ptp->num_lports = params->num_phys * ptp->ports_per_phy;
> >
> >  	ice_sb_access_ena_eth56g(hw, true);
> > -	for (phy = 0; phy < params->num_phys; phy++) {
> > -		u32 phy_rev;
> > -		int err;
> > -
> > -		err = ice_read_phy_eth56g(hw, phy, PHY_REG_REVISION,
> &phy_rev);
> > -		if (err || phy_rev != PHY_REVISION_ETH56G) {
> > -			ptp->phy_model = ICE_PHY_UNSUP;
> > -			return;
> > -		}
> > -	}
> > +	err = ice_read_phy_eth56g(hw, hw->pf_id, PHY_REG_REVISION,
> &phy_rev);
> > +	if (err || phy_rev != PHY_REVISION_ETH56G)
> > +		ptp->phy_model = ICE_PHY_UNSUP;
> >
> 
> Shouldn't this return like it did above?
> 

You're correct. Also, as you noticed in patch 4/4 of this series, in current
Implementation, where PHY type is derived from mac_type, above checker
seems to be redundant. To be fixed in v3

> >  	ptp->is_2x50g_muxed_topo = ice_is_muxed_topo(hw);
> >  }
> 
> > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> > index 0852a34ade91..35141198f261 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> > @@ -326,7 +326,8 @@ extern const struct ice_vernier_info_e82x
> e822_vernier[NUM_ICE_PTP_LNK_SPD];
> >   */
> >  #define ICE_E810_PLL_FREQ		812500000
> >  #define ICE_PTP_NOMINAL_INCVAL_E810	0x13b13b13bULL
> > -#define E810_OUT_PROP_DELAY_NS 1
> > +#define ICE_E810_OUT_PROP_DELAY_NS	1
> > +#define ICE_E825_SYNC_DELAY		6
> >
> 
> Why is this hunk in this patch?

Looks like some leftover after rebase. To be removed in next version

[Intel-wired-lan] [PATCH v2 iwl-net 2/4] ice: Fix quad registers read on E825

[Karol Kolacinski]

Quad registers are read/written incorrectly. E825 devices always use
quad 0 address and differentiate between the PHYs by changing SBQ
destination device (phy_0 or phy_0_peer).

Add helpers for reading/writing PTP registers shared per quad and use
correct quad address and SBQ destination device based on port.

Rename rmn_0 to phy_0 and remove rmn_1 and rmn_2 as E82X HW does not
support it. Rename eth56g_phy_1 to phy_0_peer.

Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
---
V1 -> V2: Fixed kdoc issues

 drivers/net/ethernet/intel/ice/ice_common.c   |   2 +-
 .../net/ethernet/intel/ice/ice_ptp_consts.h   |  75 ++----
 drivers/net/ethernet/intel/ice/ice_ptp_hw.c   | 237 +++++++++++-------
 drivers/net/ethernet/intel/ice/ice_ptp_hw.h   |  37 ++-
 drivers/net/ethernet/intel/ice/ice_sbq_cmd.h  |   7 +-
 drivers/net/ethernet/intel/ice/ice_type.h     |   1 -
 6 files changed, 177 insertions(+), 182 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 009716a12a26..0eb7f828ed3a 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -3454,7 +3454,7 @@ int ice_aq_get_fec_stats(struct ice_hw *hw, u16 pcs_quad, u16 pcs_port,
 	msg.msg_addr_low = lower_16_bits(reg_offset);
 	msg.msg_addr_high = receiver_id;
 	msg.opcode = ice_sbq_msg_rd;
-	msg.dest_dev = rmn_0;
+	msg.dest_dev = phy_0;
 
 	err = ice_sbq_rw_reg(hw, &msg, flag);
 	if (err)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_consts.h b/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
index e6980b94a6c1..e63f2a36eabf 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
@@ -10,70 +10,25 @@
 /* Constants defined for the PTP 1588 clock hardware. */
 
 const struct ice_phy_reg_info_eth56g eth56g_phy_res[NUM_ETH56G_PHY_RES] = {
-	/* ETH56G_PHY_REG_PTP */
-	{
-		/* base_addr */
-		{
-			0x092000,
-			0x126000,
-			0x1BA000,
-			0x24E000,
-			0x2E2000,
-		},
-		/* step */
-		0x98,
+	[ETH56G_PHY_REG_PTP] = {
+		.base_addr = 0x092000,
+		.step = 0x98,
 	},
-	/* ETH56G_PHY_MEM_PTP */
-	{
-		/* base_addr */
-		{
-			0x093000,
-			0x127000,
-			0x1BB000,
-			0x24F000,
-			0x2E3000,
-		},
-		/* step */
-		0x200,
+	[ETH56G_PHY_MEM_PTP] = {
+		.base_addr = 0x093000,
+		.step = 0x200,
 	},
-	/* ETH56G_PHY_REG_XPCS */
-	{
-		/* base_addr */
-		{
-			0x000000,
-			0x009400,
-			0x128000,
-			0x1BC000,
-			0x250000,
-		},
-		/* step */
-		0x21000,
+	[ETH56G_PHY_REG_XPCS] = {
+		.base_addr = 0x000000,
+		.step = 0x21000,
 	},
-	/* ETH56G_PHY_REG_MAC */
-	{
-		/* base_addr */
-		{
-			0x085000,
-			0x119000,
-			0x1AD000,
-			0x241000,
-			0x2D5000,
-		},
-		/* step */
-		0x1000,
+	[ETH56G_PHY_REG_MAC] = {
+		.base_addr = 0x085000,
+		.step = 0x1000,
 	},
-	/* ETH56G_PHY_REG_GPCS */
-	{
-		/* base_addr */
-		{
-			0x084000,
-			0x118000,
-			0x1AC000,
-			0x240000,
-			0x2D4000,
-		},
-		/* step */
-		0x400,
+	[ETH56G_PHY_REG_GPCS] = {
+		.base_addr = 0x084000,
+		.step = 0x400,
 	},
 };
 
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index d9a3c7de0342..f28d0357c88f 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -879,31 +879,46 @@ static void ice_ptp_exec_tmr_cmd(struct ice_hw *hw)
  * The following functions operate on devices with the ETH 56G PHY.
  */
 
+/**
+ * ice_ptp_get_dest_dev_e825 - get destination PHY for given port number
+ * @hw: pointer to the HW struct
+ * @port: destination port
+ *
+ * Return: destination sideband queue PHY device.
+ */
+static enum ice_sbq_msg_dev ice_ptp_get_dest_dev_e825(struct ice_hw *hw,
+						      u8 port)
+{
+	/* On a single complex E825, PHY 0 is always destination device phy_0
+	 * and PHY 1 is phy_0_peer.
+	 */
+	if (port >= hw->ptp.ports_per_phy)
+		return phy_0_peer;
+	else
+		return phy_0;
+}
+
 /**
  * ice_write_phy_eth56g - Write a PHY port register
  * @hw: pointer to the HW struct
- * @phy_idx: PHY index
+ * @port: destination port
  * @addr: PHY register address
  * @val: Value to write
  *
- * Return: 0 on success, other error codes when failed to write to PHY
+ * Return: 0 on success, other error codes when failed to write to PHY.
  */
-static int ice_write_phy_eth56g(struct ice_hw *hw, u8 phy_idx, u32 addr,
-				u32 val)
+static int ice_write_phy_eth56g(struct ice_hw *hw, u8 port, u32 addr, u32 val)
 {
-	struct ice_sbq_msg_input phy_msg;
+	struct ice_sbq_msg_input msg = {
+		.dest_dev = ice_ptp_get_dest_dev_e825(hw, port),
+		.opcode = ice_sbq_msg_wr,
+		.msg_addr_low = lower_32_bits(addr),
+		.msg_addr_high = upper_32_bits(addr),
+		.data = val
+	};
 	int err;
 
-	phy_msg.opcode = ice_sbq_msg_wr;
-
-	phy_msg.msg_addr_low = lower_16_bits(addr);
-	phy_msg.msg_addr_high = upper_16_bits(addr);
-
-	phy_msg.data = val;
-	phy_msg.dest_dev = hw->ptp.phy.eth56g.phy_addr[phy_idx];
-
-	err = ice_sbq_rw_reg(hw, &phy_msg, ICE_AQ_FLAG_RD);
-
+	err = ice_sbq_rw_reg(hw, &msg, ICE_AQ_FLAG_RD);
 	if (err)
 		ice_debug(hw, ICE_DBG_PTP, "PTP failed to send msg to phy %d\n",
 			  err);
@@ -914,41 +929,36 @@ static int ice_write_phy_eth56g(struct ice_hw *hw, u8 phy_idx, u32 addr,
 /**
  * ice_read_phy_eth56g - Read a PHY port register
  * @hw: pointer to the HW struct
- * @phy_idx: PHY index
+ * @port: destination port
  * @addr: PHY register address
  * @val: Value to write
  *
- * Return: 0 on success, other error codes when failed to read from PHY
+ * Return: 0 on success, other error codes when failed to read from PHY.
  */
-static int ice_read_phy_eth56g(struct ice_hw *hw, u8 phy_idx, u32 addr,
-			       u32 *val)
+static int ice_read_phy_eth56g(struct ice_hw *hw, u8 port, u32 addr, u32 *val)
 {
-	struct ice_sbq_msg_input phy_msg;
+	struct ice_sbq_msg_input msg = {
+		.dest_dev = ice_ptp_get_dest_dev_e825(hw, port),
+		.opcode = ice_sbq_msg_rd,
+		.msg_addr_low = lower_32_bits(addr),
+		.msg_addr_high = upper_32_bits(addr)
+	};
 	int err;
 
-	phy_msg.opcode = ice_sbq_msg_rd;
-
-	phy_msg.msg_addr_low = lower_16_bits(addr);
-	phy_msg.msg_addr_high = upper_16_bits(addr);
-
-	phy_msg.data = 0;
-	phy_msg.dest_dev = hw->ptp.phy.eth56g.phy_addr[phy_idx];
-
-	err = ice_sbq_rw_reg(hw, &phy_msg, ICE_AQ_FLAG_RD);
-	if (err) {
+	err = ice_sbq_rw_reg(hw, &msg, ICE_AQ_FLAG_RD);
+	if (err)
 		ice_debug(hw, ICE_DBG_PTP, "PTP failed to send msg to phy %d\n",
 			  err);
-		return err;
-	}
-
-	*val = phy_msg.data;
+	else
+		*val = msg.data;
 
-	return 0;
+	return err;
 }
 
 /**
- * ice_phy_res_address_eth56g - Calculate a PHY port register address
- * @port: Port number to be written
+ * ice_phy_res_addr_eth56g - Calculate a PHY port register address
+ * @hw: pointer to the HW struct
+ * @lane: Lane number to be written
  * @res_type: resource type (register/memory)
  * @offset: Offset from PHY port register base
  * @addr: The result address
@@ -957,17 +967,18 @@ static int ice_read_phy_eth56g(struct ice_hw *hw, u8 phy_idx, u32 addr,
  * * %0      - success
  * * %EINVAL - invalid port number or resource type
  */
-static int ice_phy_res_address_eth56g(u8 port, enum eth56g_res_type res_type,
-				      u32 offset, u32 *addr)
+static int ice_phy_res_addr_eth56g(struct ice_hw *hw, u8 lane,
+				   enum eth56g_res_type res_type, u32 offset,
+				   u32 *addr)
 {
-	u8 lane = port % ICE_PORTS_PER_QUAD;
-	u8 phy = ICE_GET_QUAD_NUM(port);
-
 	if (res_type >= NUM_ETH56G_PHY_RES)
 		return -EINVAL;
 
-	*addr = eth56g_phy_res[res_type].base[phy] +
+	/* Lanes 4..7 are in fact 0..3 on a second PHY */
+	lane %= hw->ptp.ports_per_phy;
+	*addr = eth56g_phy_res[res_type].base_addr +
 		lane * eth56g_phy_res[res_type].step + offset;
+
 	return 0;
 }
 
@@ -987,19 +998,17 @@ static int ice_phy_res_address_eth56g(u8 port, enum eth56g_res_type res_type,
 static int ice_write_port_eth56g(struct ice_hw *hw, u8 port, u32 offset,
 				 u32 val, enum eth56g_res_type res_type)
 {
-	u8 phy_port = port % hw->ptp.ports_per_phy;
-	u8 phy_idx = port / hw->ptp.ports_per_phy;
 	u32 addr;
 	int err;
 
 	if (port >= hw->ptp.num_lports)
 		return -EINVAL;
 
-	err = ice_phy_res_address_eth56g(phy_port, res_type, offset, &addr);
+	err = ice_phy_res_addr_eth56g(hw, port, res_type, offset, &addr);
 	if (err)
 		return err;
 
-	return ice_write_phy_eth56g(hw, phy_idx, addr, val);
+	return ice_write_phy_eth56g(hw, port, addr, val);
 }
 
 /**
@@ -1018,19 +1027,17 @@ static int ice_write_port_eth56g(struct ice_hw *hw, u8 port, u32 offset,
 static int ice_read_port_eth56g(struct ice_hw *hw, u8 port, u32 offset,
 				u32 *val, enum eth56g_res_type res_type)
 {
-	u8 phy_port = port % hw->ptp.ports_per_phy;
-	u8 phy_idx = port / hw->ptp.ports_per_phy;
 	u32 addr;
 	int err;
 
 	if (port >= hw->ptp.num_lports)
 		return -EINVAL;
 
-	err = ice_phy_res_address_eth56g(phy_port, res_type, offset, &addr);
+	err = ice_phy_res_addr_eth56g(hw, port, res_type, offset, &addr);
 	if (err)
 		return err;
 
-	return ice_read_phy_eth56g(hw, phy_idx, addr, val);
+	return ice_read_phy_eth56g(hw, port, addr, val);
 }
 
 /**
@@ -1179,6 +1186,56 @@ static int ice_write_port_mem_eth56g(struct ice_hw *hw, u8 port, u16 offset,
 	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
+ * @offset: PHY register offset
+ * @port: Port number
+ * @val: Value to write
+ *
+ * Return:
+ * * %0     - success
+ * * %EIO  - invalid port number or resource type
+ * * %other - failed to write to PHY
+ */
+static int ice_write_quad_ptp_reg_eth56g(struct ice_hw *hw, u8 port,
+					 u32 offset, u32 val)
+{
+	u32 addr;
+
+	if (port >= hw->ptp.num_lports)
+		return -EIO;
+
+	addr = eth56g_phy_res[ETH56G_PHY_REG_PTP].base_addr + offset;
+
+	return ice_write_phy_eth56g(hw, port, addr, val);
+}
+
+/**
+ * ice_read_quad_ptp_reg_eth56g - Read a PHY quad register
+ * @hw: pointer to the HW struct
+ * @offset: PHY register offset
+ * @port: Port number
+ * @val: Value to read
+ *
+ * Return:
+ * * %0     - success
+ * * %EIO  - invalid port number or resource type
+ * * %other - failed to read from PHY
+ */
+static int ice_read_quad_ptp_reg_eth56g(struct ice_hw *hw, u8 port,
+					u32 offset, u32 *val)
+{
+	u32 addr;
+
+	if (port >= hw->ptp.num_lports)
+		return -EIO;
+
+	addr = eth56g_phy_res[ETH56G_PHY_REG_PTP].base_addr + offset;
+
+	return ice_read_phy_eth56g(hw, port, addr, val);
+}
+
 /**
  * ice_is_64b_phy_reg_eth56g - Check if this is a 64bit PHY register
  * @low_addr: the low address to check
@@ -1897,7 +1954,6 @@ ice_phy_get_speed_eth56g(struct ice_link_status *li)
  */
 static int ice_phy_cfg_parpcs_eth56g(struct ice_hw *hw, u8 port)
 {
-	u8 port_blk = port & ~(ICE_PORTS_PER_QUAD - 1);
 	u32 val;
 	int err;
 
@@ -1912,8 +1968,8 @@ static int ice_phy_cfg_parpcs_eth56g(struct ice_hw *hw, u8 port)
 	switch (ice_phy_get_speed_eth56g(&hw->port_info->phy.link_info)) {
 	case ICE_ETH56G_LNK_SPD_1G:
 	case ICE_ETH56G_LNK_SPD_2_5G:
-		err = ice_read_ptp_reg_eth56g(hw, port_blk,
-					      PHY_GPCS_CONFIG_REG0, &val);
+		err = ice_read_quad_ptp_reg_eth56g(hw, port,
+						   PHY_GPCS_CONFIG_REG0, &val);
 		if (err) {
 			ice_debug(hw, ICE_DBG_PTP, "Failed to read PHY_GPCS_CONFIG_REG0, status: %d",
 				  err);
@@ -1924,8 +1980,8 @@ static int ice_phy_cfg_parpcs_eth56g(struct ice_hw *hw, u8 port)
 		val |= FIELD_PREP(PHY_GPCS_CONFIG_REG0_TX_THR_M,
 				  ICE_ETH56G_NOMINAL_TX_THRESH);
 
-		err = ice_write_ptp_reg_eth56g(hw, port_blk,
-					       PHY_GPCS_CONFIG_REG0, val);
+		err = ice_write_quad_ptp_reg_eth56g(hw, port,
+						    PHY_GPCS_CONFIG_REG0, val);
 		if (err) {
 			ice_debug(hw, ICE_DBG_PTP, "Failed to write PHY_GPCS_CONFIG_REG0, status: %d",
 				  err);
@@ -1966,50 +2022,48 @@ static int ice_phy_cfg_parpcs_eth56g(struct ice_hw *hw, u8 port)
  */
 int ice_phy_cfg_ptp_1step_eth56g(struct ice_hw *hw, u8 port)
 {
-	u8 port_blk = port & ~(ICE_PORTS_PER_QUAD - 1);
-	u8 blk_port = port & (ICE_PORTS_PER_QUAD - 1);
+	u8 quad_lane = port % ICE_PORTS_PER_QUAD;
+	u32 addr, val, peer_delay;
 	bool enable, sfd_ena;
-	u32 val, peer_delay;
 	int err;
 
 	enable = hw->ptp.phy.eth56g.onestep_ena;
 	peer_delay = hw->ptp.phy.eth56g.peer_delay;
 	sfd_ena = hw->ptp.phy.eth56g.sfd_ena;
 
-	/* PHY_PTP_1STEP_CONFIG */
-	err = ice_read_ptp_reg_eth56g(hw, port_blk, PHY_PTP_1STEP_CONFIG, &val);
+	addr = PHY_PTP_1STEP_CONFIG;
+	err = ice_read_quad_ptp_reg_eth56g(hw, port, addr, &val);
 	if (err)
 		return err;
 
 	if (enable)
-		val |= blk_port;
+		val |= BIT(quad_lane);
 	else
-		val &= ~blk_port;
+		val &= ~BIT(quad_lane);
 
-	val &= ~(PHY_PTP_1STEP_T1S_UP64_M | PHY_PTP_1STEP_T1S_DELTA_M);
+	val &= ~PHY_PTP_1STEP_T1S_UP64_M;
+	val &= ~PHY_PTP_1STEP_T1S_DELTA_M;
 
-	err = ice_write_ptp_reg_eth56g(hw, port_blk, PHY_PTP_1STEP_CONFIG, val);
+	err = ice_write_quad_ptp_reg_eth56g(hw, port, addr, val);
 	if (err)
 		return err;
 
-	/* PHY_PTP_1STEP_PEER_DELAY */
+	addr = PHY_PTP_1STEP_PEER_DELAY(quad_lane);
 	val = FIELD_PREP(PHY_PTP_1STEP_PD_DELAY_M, peer_delay);
 	if (peer_delay)
 		val |= PHY_PTP_1STEP_PD_ADD_PD_M;
 	val |= PHY_PTP_1STEP_PD_DLY_V_M;
-	err = ice_write_ptp_reg_eth56g(hw, port_blk,
-				       PHY_PTP_1STEP_PEER_DELAY(blk_port), val);
+	err = ice_write_quad_ptp_reg_eth56g(hw, port, addr, val);
 	if (err)
 		return err;
 
 	val &= ~PHY_PTP_1STEP_PD_DLY_V_M;
-	err = ice_write_ptp_reg_eth56g(hw, port_blk,
-				       PHY_PTP_1STEP_PEER_DELAY(blk_port), val);
+	err = ice_write_quad_ptp_reg_eth56g(hw, port, addr, val);
 	if (err)
 		return err;
 
-	/* PHY_MAC_XIF_MODE */
-	err = ice_read_mac_reg_eth56g(hw, port, PHY_MAC_XIF_MODE, &val);
+	addr = PHY_MAC_XIF_MODE;
+	err = ice_read_mac_reg_eth56g(hw, port, addr, &val);
 	if (err)
 		return err;
 
@@ -2029,7 +2083,7 @@ int ice_phy_cfg_ptp_1step_eth56g(struct ice_hw *hw, u8 port)
 	       FIELD_PREP(PHY_MAC_XIF_TS_BIN_MODE_M, enable) |
 	       FIELD_PREP(PHY_MAC_XIF_TS_SFD_ENA_M, sfd_ena);
 
-	return ice_write_mac_reg_eth56g(hw, port, PHY_MAC_XIF_MODE, val);
+	return ice_write_mac_reg_eth56g(hw, port, addr, val);
 }
 
 /**
@@ -2071,21 +2125,22 @@ static u32 ice_ptp_calc_bitslip_eth56g(struct ice_hw *hw, u8 port, u32 bs,
 				       bool fc, bool rs,
 				       enum ice_eth56g_link_spd spd)
 {
-	u8 port_offset = port & (ICE_PORTS_PER_QUAD - 1);
-	u8 port_blk = port & ~(ICE_PORTS_PER_QUAD - 1);
 	u32 bitslip;
 	int err;
 
 	if (!bs || rs)
 		return 0;
 
-	if (spd == ICE_ETH56G_LNK_SPD_1G || spd == ICE_ETH56G_LNK_SPD_2_5G)
+	if (spd == ICE_ETH56G_LNK_SPD_1G || spd == ICE_ETH56G_LNK_SPD_2_5G) {
 		err = ice_read_gpcs_reg_eth56g(hw, port, PHY_GPCS_BITSLIP,
 					       &bitslip);
-	else
-		err = ice_read_ptp_reg_eth56g(hw, port_blk,
-					      PHY_REG_SD_BIT_SLIP(port_offset),
-					      &bitslip);
+	} else {
+		u8 quad_lane = port % ICE_PORTS_PER_QUAD;
+		u32 addr;
+
+		addr = PHY_REG_SD_BIT_SLIP(quad_lane);
+		err = ice_read_quad_ptp_reg_eth56g(hw, port, addr, &bitslip);
+	}
 	if (err)
 		return 0;
 
@@ -2554,9 +2609,9 @@ static void ice_sb_access_ena_eth56g(struct ice_hw *hw, bool enable)
 	u32 val = rd32(hw, PF_SB_REM_DEV_CTL);
 
 	if (enable)
-		val |= BIT(eth56g_phy_0) | BIT(cgu) | BIT(eth56g_phy_1);
+		val |= BIT(phy_0) | BIT(cgu) | BIT(phy_0_peer);
 	else
-		val &= ~(BIT(eth56g_phy_0) | BIT(cgu) | BIT(eth56g_phy_1));
+		val &= ~(BIT(phy_0) | BIT(cgu) | BIT(phy_0_peer));
 
 	wr32(hw, PF_SB_REM_DEV_CTL, val);
 }
@@ -2680,8 +2735,6 @@ static void ice_ptp_init_phy_e825(struct ice_hw *hw)
 	params->onestep_ena = false;
 	params->peer_delay = 0;
 	params->sfd_ena = false;
-	params->phy_addr[0] = eth56g_phy_0;
-	params->phy_addr[1] = eth56g_phy_1;
 	params->num_phys = 2;
 	ptp->ports_per_phy = 4;
 	ptp->num_lports = params->num_phys * ptp->ports_per_phy;
@@ -2710,10 +2763,9 @@ static void ice_fill_phy_msg_e82x(struct ice_hw *hw,
 				  struct ice_sbq_msg_input *msg, u8 port,
 				  u16 offset)
 {
-	int phy_port, phy, quadtype;
+	int phy_port, quadtype;
 
 	phy_port = port % hw->ptp.ports_per_phy;
-	phy = port / hw->ptp.ports_per_phy;
 	quadtype = ICE_GET_QUAD_NUM(port) %
 		   ICE_GET_QUAD_NUM(hw->ptp.ports_per_phy);
 
@@ -2725,12 +2777,7 @@ static void ice_fill_phy_msg_e82x(struct ice_hw *hw,
 		msg->msg_addr_high = P_Q1_H(P_4_BASE + offset, phy_port);
 	}
 
-	if (phy == 0)
-		msg->dest_dev = rmn_0;
-	else if (phy == 1)
-		msg->dest_dev = rmn_1;
-	else
-		msg->dest_dev = rmn_2;
+	msg->dest_dev = phy_0;
 }
 
 /**
@@ -3053,7 +3100,7 @@ static int ice_fill_quad_msg_e82x(struct ice_hw *hw,
 	if (quad >= ICE_GET_QUAD_NUM(hw->ptp.num_lports))
 		return -EINVAL;
 
-	msg->dest_dev = rmn_0;
+	msg->dest_dev = phy_0;
 
 	if (!(quad % ICE_GET_QUAD_NUM(hw->ptp.ports_per_phy)))
 		addr = Q_0_BASE + offset;
@@ -4772,7 +4819,7 @@ static int ice_read_phy_reg_e810(struct ice_hw *hw, u32 addr, u32 *val)
 	msg.msg_addr_low = lower_16_bits(addr);
 	msg.msg_addr_high = upper_16_bits(addr);
 	msg.opcode = ice_sbq_msg_rd;
-	msg.dest_dev = rmn_0;
+	msg.dest_dev = phy_0;
 
 	err = ice_sbq_rw_reg(hw, &msg, ICE_AQ_FLAG_RD);
 	if (err) {
@@ -4802,7 +4849,7 @@ static int ice_write_phy_reg_e810(struct ice_hw *hw, u32 addr, u32 val)
 	msg.msg_addr_low = lower_16_bits(addr);
 	msg.msg_addr_high = upper_16_bits(addr);
 	msg.opcode = ice_sbq_msg_wr;
-	msg.dest_dev = rmn_0;
+	msg.dest_dev = phy_0;
 	msg.data = val;
 
 	err = ice_sbq_rw_reg(hw, &msg, ICE_AQ_FLAG_RD);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
index 35141198f261..d913ed00c730 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
@@ -65,14 +65,14 @@ enum ice_eth56g_link_spd {
 
 /**
  * struct ice_phy_reg_info_eth56g - ETH56G PHY register parameters
- * @base: base address for each PHY block
+ * @base_addr: base address for each PHY block
  * @step: step between PHY lanes
  *
  * Characteristic information for the various PHY register parameters in the
  * ETH56G devices
  */
 struct ice_phy_reg_info_eth56g {
-	u32 base[NUM_ETH56G_PHY_RES];
+	u32 base_addr;
 	u32 step;
 };
 
@@ -773,37 +773,20 @@ static inline u64 ice_get_base_incval(struct ice_hw *hw)
 #define PHY_MAC_XIF_TS_SFD_ENA_M	ICE_M(0x1, 20)
 #define PHY_MAC_XIF_GMII_TS_SEL_M	ICE_M(0x1, 21)
 
-/* GPCS config register */
-#define PHY_GPCS_CONFIG_REG0		0x268
-#define PHY_GPCS_CONFIG_REG0_TX_THR_M	ICE_M(0xF, 24)
-#define PHY_GPCS_BITSLIP		0x5C
-
 #define PHY_TS_INT_CONFIG_THRESHOLD_M	ICE_M(0x3F, 0)
 #define PHY_TS_INT_CONFIG_ENA_M		BIT(6)
 
-/* 1-step PTP config */
-#define PHY_PTP_1STEP_CONFIG		0x270
-#define PHY_PTP_1STEP_T1S_UP64_M	ICE_M(0xF, 4)
-#define PHY_PTP_1STEP_T1S_DELTA_M	ICE_M(0xF, 8)
-#define PHY_PTP_1STEP_PEER_DELAY(_port)	(0x274 + 4 * (_port))
-#define PHY_PTP_1STEP_PD_ADD_PD_M	ICE_M(0x1, 0)
-#define PHY_PTP_1STEP_PD_DELAY_M	ICE_M(0x3fffffff, 1)
-#define PHY_PTP_1STEP_PD_DLY_V_M	ICE_M(0x1, 31)
-
 /* Macros to derive offsets for TimeStampLow and TimeStampHigh */
 #define PHY_TSTAMP_L(x) (((x) * 8) + 0)
 #define PHY_TSTAMP_U(x) (((x) * 8) + 4)
 
-#define PHY_REG_REVISION		0x85000
-
 #define PHY_REG_DESKEW_0		0x94
 #define PHY_REG_DESKEW_0_RLEVEL		GENMASK(6, 0)
 #define PHY_REG_DESKEW_0_RLEVEL_FRAC	GENMASK(9, 7)
 #define PHY_REG_DESKEW_0_RLEVEL_FRAC_W	3
 #define PHY_REG_DESKEW_0_VALID		GENMASK(10, 10)
 
-#define PHY_REG_GPCS_BITSLIP		0x5C
-#define PHY_REG_SD_BIT_SLIP(_port_offset)	(0x29C + 4 * (_port_offset))
+#define PHY_REG_REVISION		0x85000
 #define PHY_REVISION_ETH56G		0x10200
 #define PHY_VENDOR_TXLANE_THRESH	0x2000C
 
@@ -822,7 +805,21 @@ static inline u64 ice_get_base_incval(struct ice_hw *hw)
 #define PHY_MAC_BLOCKTIME		0x50
 #define PHY_MAC_MARKERTIME		0x54
 #define PHY_MAC_TX_OFFSET		0x58
+#define PHY_GPCS_BITSLIP		0x5C
 
 #define PHY_PTP_INT_STATUS		0x7FD140
 
+/* ETH56G registers shared per quad */
+/* GPCS config register */
+#define PHY_GPCS_CONFIG_REG0		0x268
+#define PHY_GPCS_CONFIG_REG0_TX_THR_M	GENMASK(27, 24)
+/* 1-step PTP config */
+#define PHY_PTP_1STEP_CONFIG		0x270
+#define PHY_PTP_1STEP_T1S_UP64_M	GENMASK(7, 4)
+#define PHY_PTP_1STEP_T1S_DELTA_M	GENMASK(11, 8)
+#define PHY_PTP_1STEP_PEER_DELAY(_quad_lane)	(0x274 + 4 * (_quad_lane))
+#define PHY_PTP_1STEP_PD_ADD_PD_M	BIT(0)
+#define PHY_PTP_1STEP_PD_DELAY_M	GENMASK(30, 1)
+#define PHY_PTP_1STEP_PD_DLY_V_M	BIT(31)
+#define PHY_REG_SD_BIT_SLIP(_quad_lane)	(0x29C + 4 * (_quad_lane))
 #endif /* _ICE_PTP_HW_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h b/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
index 3b0054faf70c..f362607ece35 100644
--- a/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
@@ -47,12 +47,9 @@ struct ice_sbq_evt_desc {
 };
 
 enum ice_sbq_msg_dev {
-	eth56g_phy_0	= 0x02,
-	rmn_0		= 0x02,
-	rmn_1		= 0x03,
-	rmn_2		= 0x04,
+	phy_0		= 0x02,
 	cgu		= 0x06,
-	eth56g_phy_1	= 0x0D,
+	phy_0_peer	= 0x0D,
 };
 
 enum ice_sbq_msg_opcode {
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index 45768796691f..479227bdff75 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -850,7 +850,6 @@ struct ice_mbx_data {
 
 struct ice_eth56g_params {
 	u8 num_phys;
-	u8 phy_addr[2];
 	bool onestep_ena;
 	bool sfd_ena;
 	u32 peer_delay;
-- 
2.46.2

Re: [Intel-wired-lan] [PATCH v2 iwl-net 2/4] ice: Fix quad registers read on E825

[Nitka, Grzegorz]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Karol Kolacinski
> Sent: Thursday, October 10, 2024 4:21 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Kolacinski, Karol <karol.kolacinski@intel.com>;
> Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>; Nguyen, Anthony
> L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: [Intel-wired-lan] [PATCH v2 iwl-net 2/4] ice: Fix quad registers read
> on E825
> 
> Quad registers are read/written incorrectly. E825 devices always use
> quad 0 address and differentiate between the PHYs by changing SBQ
> destination device (phy_0 or phy_0_peer).
> 
> Add helpers for reading/writing PTP registers shared per quad and use
> correct quad address and SBQ destination device based on port.
> 
> Rename rmn_0 to phy_0 and remove rmn_1 and rmn_2 as E82X HW does not
> support it. Rename eth56g_phy_1 to phy_0_peer.
> 
> Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
> ---
> V1 -> V2: Fixed kdoc issues
> 
>  drivers/net/ethernet/intel/ice/ice_common.c   |   2 +-
>  .../net/ethernet/intel/ice/ice_ptp_consts.h   |  75 ++----
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.c   | 237 +++++++++++-------
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.h   |  37 ++-
>  drivers/net/ethernet/intel/ice/ice_sbq_cmd.h  |   7 +-
>  drivers/net/ethernet/intel/ice/ice_type.h     |   1 -
>  6 files changed, 177 insertions(+), 182 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
> b/drivers/net/ethernet/intel/ice/ice_common.c
> index 009716a12a26..0eb7f828ed3a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -3454,7 +3454,7 @@ int ice_aq_get_fec_stats(struct ice_hw *hw, u16
> pcs_quad, u16 pcs_port,
>  	msg.msg_addr_low = lower_16_bits(reg_offset);
>  	msg.msg_addr_high = receiver_id;
>  	msg.opcode = ice_sbq_msg_rd;
> -	msg.dest_dev = rmn_0;
> +	msg.dest_dev = phy_0;
> 
>  	err = ice_sbq_rw_reg(hw, &msg, flag);
>  	if (err)
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
> b/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
> index e6980b94a6c1..e63f2a36eabf 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
> @@ -10,70 +10,25 @@
>  /* Constants defined for the PTP 1588 clock hardware. */
> 
>  const struct ice_phy_reg_info_eth56g
> eth56g_phy_res[NUM_ETH56G_PHY_RES] = {
> -	/* ETH56G_PHY_REG_PTP */
> -	{
> -		/* base_addr */
> -		{
> -			0x092000,
> -			0x126000,
> -			0x1BA000,
> -			0x24E000,
> -			0x2E2000,
> -		},
> -		/* step */
> -		0x98,
> +	[ETH56G_PHY_REG_PTP] = {
> +		.base_addr = 0x092000,
> +		.step = 0x98,
>  	},
> -	/* ETH56G_PHY_MEM_PTP */
> -	{
> -		/* base_addr */
> -		{
> -			0x093000,
> -			0x127000,
> -			0x1BB000,
> -			0x24F000,
> -			0x2E3000,
> -		},
> -		/* step */
> -		0x200,
> +	[ETH56G_PHY_MEM_PTP] = {
> +		.base_addr = 0x093000,
> +		.step = 0x200,
>  	},
> -	/* ETH56G_PHY_REG_XPCS */
> -	{
> -		/* base_addr */
> -		{
> -			0x000000,
> -			0x009400,
> -			0x128000,
> -			0x1BC000,
> -			0x250000,
> -		},
> -		/* step */
> -		0x21000,
> +	[ETH56G_PHY_REG_XPCS] = {
> +		.base_addr = 0x000000,
> +		.step = 0x21000,
>  	},
> -	/* ETH56G_PHY_REG_MAC */
> -	{
> -		/* base_addr */
> -		{
> -			0x085000,
> -			0x119000,
> -			0x1AD000,
> -			0x241000,
> -			0x2D5000,
> -		},
> -		/* step */
> -		0x1000,
> +	[ETH56G_PHY_REG_MAC] = {
> +		.base_addr = 0x085000,
> +		.step = 0x1000,
>  	},
> -	/* ETH56G_PHY_REG_GPCS */
> -	{
> -		/* base_addr */
> -		{
> -			0x084000,
> -			0x118000,
> -			0x1AC000,
> -			0x240000,
> -			0x2D4000,
> -		},
> -		/* step */
> -		0x400,
> +	[ETH56G_PHY_REG_GPCS] = {
> +		.base_addr = 0x084000,
> +		.step = 0x400,
>  	},
>  };
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index d9a3c7de0342..f28d0357c88f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -879,31 +879,46 @@ static void ice_ptp_exec_tmr_cmd(struct ice_hw
> *hw)
>   * The following functions operate on devices with the ETH 56G PHY.
>   */
> 
> +/**
> + * ice_ptp_get_dest_dev_e825 - get destination PHY for given port number
> + * @hw: pointer to the HW struct
> + * @port: destination port
> + *
> + * Return: destination sideband queue PHY device.
> + */
> +static enum ice_sbq_msg_dev ice_ptp_get_dest_dev_e825(struct ice_hw
> *hw,
> +						      u8 port)
> +{
> +	/* On a single complex E825, PHY 0 is always destination device phy_0
> +	 * and PHY 1 is phy_0_peer.
> +	 */
> +	if (port >= hw->ptp.ports_per_phy)
> +		return phy_0_peer;
> +	else
> +		return phy_0;
> +}
> +
>  /**
>   * ice_write_phy_eth56g - Write a PHY port register
>   * @hw: pointer to the HW struct
> - * @phy_idx: PHY index
> + * @port: destination port
>   * @addr: PHY register address
>   * @val: Value to write
>   *
> - * Return: 0 on success, other error codes when failed to write to PHY
> + * Return: 0 on success, other error codes when failed to write to PHY.
>   */
> -static int ice_write_phy_eth56g(struct ice_hw *hw, u8 phy_idx, u32 addr,
> -				u32 val)
> +static int ice_write_phy_eth56g(struct ice_hw *hw, u8 port, u32 addr, u32
> val)
>  {
> -	struct ice_sbq_msg_input phy_msg;
> +	struct ice_sbq_msg_input msg = {
> +		.dest_dev = ice_ptp_get_dest_dev_e825(hw, port),
> +		.opcode = ice_sbq_msg_wr,
> +		.msg_addr_low = lower_32_bits(addr),
> +		.msg_addr_high = upper_32_bits(addr),

This needs to be lower_16_bits(addr) and upper_16_bits(addr)

> +		.data = val
> +	};
>  	int err;
> 
> -	phy_msg.opcode = ice_sbq_msg_wr;
> -
> -	phy_msg.msg_addr_low = lower_16_bits(addr);
> -	phy_msg.msg_addr_high = upper_16_bits(addr);
> -
> -	phy_msg.data = val;
> -	phy_msg.dest_dev = hw->ptp.phy.eth56g.phy_addr[phy_idx];
> -
> -	err = ice_sbq_rw_reg(hw, &phy_msg, ICE_AQ_FLAG_RD);
> -
> +	err = ice_sbq_rw_reg(hw, &msg, ICE_AQ_FLAG_RD);
>  	if (err)
>  		ice_debug(hw, ICE_DBG_PTP, "PTP failed to send msg to phy
> %d\n",
>  			  err);
> @@ -914,41 +929,36 @@ static int ice_write_phy_eth56g(struct ice_hw *hw,
> u8 phy_idx, u32 addr,
>  /**
>   * ice_read_phy_eth56g - Read a PHY port register
>   * @hw: pointer to the HW struct
> - * @phy_idx: PHY index
> + * @port: destination port
>   * @addr: PHY register address
>   * @val: Value to write
>   *
> - * Return: 0 on success, other error codes when failed to read from PHY
> + * Return: 0 on success, other error codes when failed to read from PHY.
>   */
> -static int ice_read_phy_eth56g(struct ice_hw *hw, u8 phy_idx, u32 addr,
> -			       u32 *val)
> +static int ice_read_phy_eth56g(struct ice_hw *hw, u8 port, u32 addr, u32
> *val)
>  {
> -	struct ice_sbq_msg_input phy_msg;
> +	struct ice_sbq_msg_input msg = {
> +		.dest_dev = ice_ptp_get_dest_dev_e825(hw, port),
> +		.opcode = ice_sbq_msg_rd,
> +		.msg_addr_low = lower_32_bits(addr),
> +		.msg_addr_high = upper_32_bits(addr)

Same as above, this needs to be lower_16_bits(addr) and upper_16_bits(addr)

> +	};
>  	int err;
> 
> -	phy_msg.opcode = ice_sbq_msg_rd;
> -
> -	phy_msg.msg_addr_low = lower_16_bits(addr);
> -	phy_msg.msg_addr_high = upper_16_bits(addr);
> -
> -	phy_msg.data = 0;
> -	phy_msg.dest_dev = hw->ptp.phy.eth56g.phy_addr[phy_idx];
> -
> -	err = ice_sbq_rw_reg(hw, &phy_msg, ICE_AQ_FLAG_RD);
> -	if (err) {
> +	err = ice_sbq_rw_reg(hw, &msg, ICE_AQ_FLAG_RD);
> +	if (err)
>  		ice_debug(hw, ICE_DBG_PTP, "PTP failed to send msg to phy
> %d\n",
>  			  err);
> -		return err;
> -	}
> -
> -	*val = phy_msg.data;
> +	else
> +		*val = msg.data;
> 
> -	return 0;
> +	return err;
>  }
> 
>  /**
> - * ice_phy_res_address_eth56g - Calculate a PHY port register address
> - * @port: Port number to be written
> + * ice_phy_res_addr_eth56g - Calculate a PHY port register address
> + * @hw: pointer to the HW struct
> + * @lane: Lane number to be written
>   * @res_type: resource type (register/memory)
>   * @offset: Offset from PHY port register base
>   * @addr: The result address
> @@ -957,17 +967,18 @@ static int ice_read_phy_eth56g(struct ice_hw *hw,
> u8 phy_idx, u32 addr,
>   * * %0      - success
>   * * %EINVAL - invalid port number or resource type
>   */
> -static int ice_phy_res_address_eth56g(u8 port, enum eth56g_res_type
> res_type,
> -				      u32 offset, u32 *addr)
> +static int ice_phy_res_addr_eth56g(struct ice_hw *hw, u8 lane,
> +				   enum eth56g_res_type res_type, u32
> offset,
> +				   u32 *addr)
>  {
> -	u8 lane = port % ICE_PORTS_PER_QUAD;
> -	u8 phy = ICE_GET_QUAD_NUM(port);
> -
>  	if (res_type >= NUM_ETH56G_PHY_RES)
>  		return -EINVAL;
> 
> -	*addr = eth56g_phy_res[res_type].base[phy] +
> +	/* Lanes 4..7 are in fact 0..3 on a second PHY */
> +	lane %= hw->ptp.ports_per_phy;
> +	*addr = eth56g_phy_res[res_type].base_addr +
>  		lane * eth56g_phy_res[res_type].step + offset;
> +
>  	return 0;
>  }
> 
> @@ -987,19 +998,17 @@ static int ice_phy_res_address_eth56g(u8 port,
> enum eth56g_res_type res_type,
>  static int ice_write_port_eth56g(struct ice_hw *hw, u8 port, u32 offset,
>  				 u32 val, enum eth56g_res_type res_type)
>  {
> -	u8 phy_port = port % hw->ptp.ports_per_phy;
> -	u8 phy_idx = port / hw->ptp.ports_per_phy;
>  	u32 addr;
>  	int err;
> 
>  	if (port >= hw->ptp.num_lports)
>  		return -EINVAL;
> 
> -	err = ice_phy_res_address_eth56g(phy_port, res_type, offset,
> &addr);
> +	err = ice_phy_res_addr_eth56g(hw, port, res_type, offset, &addr);
>  	if (err)
>  		return err;
> 
> -	return ice_write_phy_eth56g(hw, phy_idx, addr, val);
> +	return ice_write_phy_eth56g(hw, port, addr, val);
>  }
> 
>  /**
> @@ -1018,19 +1027,17 @@ static int ice_write_port_eth56g(struct ice_hw
> *hw, u8 port, u32 offset,
>  static int ice_read_port_eth56g(struct ice_hw *hw, u8 port, u32 offset,
>  				u32 *val, enum eth56g_res_type res_type)
>  {
> -	u8 phy_port = port % hw->ptp.ports_per_phy;
> -	u8 phy_idx = port / hw->ptp.ports_per_phy;
>  	u32 addr;
>  	int err;
> 
>  	if (port >= hw->ptp.num_lports)
>  		return -EINVAL;
> 
> -	err = ice_phy_res_address_eth56g(phy_port, res_type, offset,
> &addr);
> +	err = ice_phy_res_addr_eth56g(hw, port, res_type, offset, &addr);
>  	if (err)
>  		return err;
> 
> -	return ice_read_phy_eth56g(hw, phy_idx, addr, val);
> +	return ice_read_phy_eth56g(hw, port, addr, val);
>  }
> 
>  /**
> @@ -1179,6 +1186,56 @@ static int ice_write_port_mem_eth56g(struct
> ice_hw *hw, u8 port, u16 offset,
>  	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
> + * @offset: PHY register offset
> + * @port: Port number
> + * @val: Value to write
> + *
> + * Return:
> + * * %0     - success
> + * * %EIO  - invalid port number or resource type
> + * * %other - failed to write to PHY
> + */
> +static int ice_write_quad_ptp_reg_eth56g(struct ice_hw *hw, u8 port,
> +					 u32 offset, u32 val)
> +{
> +	u32 addr;
> +
> +	if (port >= hw->ptp.num_lports)
> +		return -EIO;
> +
> +	addr = eth56g_phy_res[ETH56G_PHY_REG_PTP].base_addr + offset;
> +
> +	return ice_write_phy_eth56g(hw, port, addr, val);
> +}
> +
> +/**
> + * ice_read_quad_ptp_reg_eth56g - Read a PHY quad register
> + * @hw: pointer to the HW struct
> + * @offset: PHY register offset
> + * @port: Port number
> + * @val: Value to read
> + *
> + * Return:
> + * * %0     - success
> + * * %EIO  - invalid port number or resource type
> + * * %other - failed to read from PHY
> + */
> +static int ice_read_quad_ptp_reg_eth56g(struct ice_hw *hw, u8 port,
> +					u32 offset, u32 *val)
> +{
> +	u32 addr;
> +
> +	if (port >= hw->ptp.num_lports)
> +		return -EIO;
> +
> +	addr = eth56g_phy_res[ETH56G_PHY_REG_PTP].base_addr + offset;
> +
> +	return ice_read_phy_eth56g(hw, port, addr, val);
> +}
> +
>  /**
>   * ice_is_64b_phy_reg_eth56g - Check if this is a 64bit PHY register
>   * @low_addr: the low address to check
> @@ -1897,7 +1954,6 @@ ice_phy_get_speed_eth56g(struct ice_link_status
> *li)
>   */
>  static int ice_phy_cfg_parpcs_eth56g(struct ice_hw *hw, u8 port)
>  {
> -	u8 port_blk = port & ~(ICE_PORTS_PER_QUAD - 1);
>  	u32 val;
>  	int err;
> 
> @@ -1912,8 +1968,8 @@ static int ice_phy_cfg_parpcs_eth56g(struct ice_hw
> *hw, u8 port)
>  	switch (ice_phy_get_speed_eth56g(&hw->port_info-
> >phy.link_info)) {
>  	case ICE_ETH56G_LNK_SPD_1G:
>  	case ICE_ETH56G_LNK_SPD_2_5G:
> -		err = ice_read_ptp_reg_eth56g(hw, port_blk,
> -					      PHY_GPCS_CONFIG_REG0, &val);
> +		err = ice_read_quad_ptp_reg_eth56g(hw, port,
> +						   PHY_GPCS_CONFIG_REG0,
> &val);
>  		if (err) {
>  			ice_debug(hw, ICE_DBG_PTP, "Failed to read
> PHY_GPCS_CONFIG_REG0, status: %d",
>  				  err);
> @@ -1924,8 +1980,8 @@ static int ice_phy_cfg_parpcs_eth56g(struct ice_hw
> *hw, u8 port)
>  		val |= FIELD_PREP(PHY_GPCS_CONFIG_REG0_TX_THR_M,
>  				  ICE_ETH56G_NOMINAL_TX_THRESH);
> 
> -		err = ice_write_ptp_reg_eth56g(hw, port_blk,
> -					       PHY_GPCS_CONFIG_REG0, val);
> +		err = ice_write_quad_ptp_reg_eth56g(hw, port,
> +						    PHY_GPCS_CONFIG_REG0,
> val);
>  		if (err) {
>  			ice_debug(hw, ICE_DBG_PTP, "Failed to write
> PHY_GPCS_CONFIG_REG0, status: %d",
>  				  err);
> @@ -1966,50 +2022,48 @@ static int ice_phy_cfg_parpcs_eth56g(struct
> ice_hw *hw, u8 port)
>   */
>  int ice_phy_cfg_ptp_1step_eth56g(struct ice_hw *hw, u8 port)
>  {
> -	u8 port_blk = port & ~(ICE_PORTS_PER_QUAD - 1);
> -	u8 blk_port = port & (ICE_PORTS_PER_QUAD - 1);
> +	u8 quad_lane = port % ICE_PORTS_PER_QUAD;
> +	u32 addr, val, peer_delay;
>  	bool enable, sfd_ena;
> -	u32 val, peer_delay;
>  	int err;
> 
>  	enable = hw->ptp.phy.eth56g.onestep_ena;
>  	peer_delay = hw->ptp.phy.eth56g.peer_delay;
>  	sfd_ena = hw->ptp.phy.eth56g.sfd_ena;
> 
> -	/* PHY_PTP_1STEP_CONFIG */
> -	err = ice_read_ptp_reg_eth56g(hw, port_blk,
> PHY_PTP_1STEP_CONFIG, &val);
> +	addr = PHY_PTP_1STEP_CONFIG;
> +	err = ice_read_quad_ptp_reg_eth56g(hw, port, addr, &val);
>  	if (err)
>  		return err;
> 
>  	if (enable)
> -		val |= blk_port;
> +		val |= BIT(quad_lane);
>  	else
> -		val &= ~blk_port;
> +		val &= ~BIT(quad_lane);
> 
> -	val &= ~(PHY_PTP_1STEP_T1S_UP64_M |
> PHY_PTP_1STEP_T1S_DELTA_M);
> +	val &= ~PHY_PTP_1STEP_T1S_UP64_M;
> +	val &= ~PHY_PTP_1STEP_T1S_DELTA_M;
> 
> -	err = ice_write_ptp_reg_eth56g(hw, port_blk,
> PHY_PTP_1STEP_CONFIG, val);
> +	err = ice_write_quad_ptp_reg_eth56g(hw, port, addr, val);
>  	if (err)
>  		return err;
> 
> -	/* PHY_PTP_1STEP_PEER_DELAY */
> +	addr = PHY_PTP_1STEP_PEER_DELAY(quad_lane);
>  	val = FIELD_PREP(PHY_PTP_1STEP_PD_DELAY_M, peer_delay);
>  	if (peer_delay)
>  		val |= PHY_PTP_1STEP_PD_ADD_PD_M;
>  	val |= PHY_PTP_1STEP_PD_DLY_V_M;
> -	err = ice_write_ptp_reg_eth56g(hw, port_blk,
> -				       PHY_PTP_1STEP_PEER_DELAY(blk_port),
> val);
> +	err = ice_write_quad_ptp_reg_eth56g(hw, port, addr, val);
>  	if (err)
>  		return err;
> 
>  	val &= ~PHY_PTP_1STEP_PD_DLY_V_M;
> -	err = ice_write_ptp_reg_eth56g(hw, port_blk,
> -				       PHY_PTP_1STEP_PEER_DELAY(blk_port),
> val);
> +	err = ice_write_quad_ptp_reg_eth56g(hw, port, addr, val);
>  	if (err)
>  		return err;
> 
> -	/* PHY_MAC_XIF_MODE */
> -	err = ice_read_mac_reg_eth56g(hw, port, PHY_MAC_XIF_MODE,
> &val);
> +	addr = PHY_MAC_XIF_MODE;
> +	err = ice_read_mac_reg_eth56g(hw, port, addr, &val);
>  	if (err)
>  		return err;
> 
> @@ -2029,7 +2083,7 @@ int ice_phy_cfg_ptp_1step_eth56g(struct ice_hw
> *hw, u8 port)
>  	       FIELD_PREP(PHY_MAC_XIF_TS_BIN_MODE_M, enable) |
>  	       FIELD_PREP(PHY_MAC_XIF_TS_SFD_ENA_M, sfd_ena);
> 
> -	return ice_write_mac_reg_eth56g(hw, port, PHY_MAC_XIF_MODE,
> val);
> +	return ice_write_mac_reg_eth56g(hw, port, addr, val);
>  }
> 
>  /**
> @@ -2071,21 +2125,22 @@ static u32 ice_ptp_calc_bitslip_eth56g(struct
> ice_hw *hw, u8 port, u32 bs,
>  				       bool fc, bool rs,
>  				       enum ice_eth56g_link_spd spd)
>  {
> -	u8 port_offset = port & (ICE_PORTS_PER_QUAD - 1);
> -	u8 port_blk = port & ~(ICE_PORTS_PER_QUAD - 1);
>  	u32 bitslip;
>  	int err;
> 
>  	if (!bs || rs)
>  		return 0;
> 
> -	if (spd == ICE_ETH56G_LNK_SPD_1G || spd ==
> ICE_ETH56G_LNK_SPD_2_5G)
> +	if (spd == ICE_ETH56G_LNK_SPD_1G || spd ==
> ICE_ETH56G_LNK_SPD_2_5G) {
>  		err = ice_read_gpcs_reg_eth56g(hw, port,
> PHY_GPCS_BITSLIP,
>  					       &bitslip);
> -	else
> -		err = ice_read_ptp_reg_eth56g(hw, port_blk,
> -
> PHY_REG_SD_BIT_SLIP(port_offset),
> -					      &bitslip);
> +	} else {
> +		u8 quad_lane = port % ICE_PORTS_PER_QUAD;
> +		u32 addr;
> +
> +		addr = PHY_REG_SD_BIT_SLIP(quad_lane);
> +		err = ice_read_quad_ptp_reg_eth56g(hw, port, addr,
> &bitslip);
> +	}
>  	if (err)
>  		return 0;
> 
> @@ -2554,9 +2609,9 @@ static void ice_sb_access_ena_eth56g(struct
> ice_hw *hw, bool enable)
>  	u32 val = rd32(hw, PF_SB_REM_DEV_CTL);
> 
>  	if (enable)
> -		val |= BIT(eth56g_phy_0) | BIT(cgu) | BIT(eth56g_phy_1);
> +		val |= BIT(phy_0) | BIT(cgu) | BIT(phy_0_peer);
>  	else
> -		val &= ~(BIT(eth56g_phy_0) | BIT(cgu) | BIT(eth56g_phy_1));
> +		val &= ~(BIT(phy_0) | BIT(cgu) | BIT(phy_0_peer));
> 
>  	wr32(hw, PF_SB_REM_DEV_CTL, val);
>  }
> @@ -2680,8 +2735,6 @@ static void ice_ptp_init_phy_e825(struct ice_hw
> *hw)
>  	params->onestep_ena = false;
>  	params->peer_delay = 0;
>  	params->sfd_ena = false;
> -	params->phy_addr[0] = eth56g_phy_0;
> -	params->phy_addr[1] = eth56g_phy_1;
>  	params->num_phys = 2;
>  	ptp->ports_per_phy = 4;
>  	ptp->num_lports = params->num_phys * ptp->ports_per_phy;
> @@ -2710,10 +2763,9 @@ static void ice_fill_phy_msg_e82x(struct ice_hw
> *hw,
>  				  struct ice_sbq_msg_input *msg, u8 port,
>  				  u16 offset)
>  {
> -	int phy_port, phy, quadtype;
> +	int phy_port, quadtype;
> 
>  	phy_port = port % hw->ptp.ports_per_phy;
> -	phy = port / hw->ptp.ports_per_phy;
>  	quadtype = ICE_GET_QUAD_NUM(port) %
>  		   ICE_GET_QUAD_NUM(hw->ptp.ports_per_phy);
> 
> @@ -2725,12 +2777,7 @@ static void ice_fill_phy_msg_e82x(struct ice_hw
> *hw,
>  		msg->msg_addr_high = P_Q1_H(P_4_BASE + offset,
> phy_port);
>  	}
> 
> -	if (phy == 0)
> -		msg->dest_dev = rmn_0;
> -	else if (phy == 1)
> -		msg->dest_dev = rmn_1;
> -	else
> -		msg->dest_dev = rmn_2;
> +	msg->dest_dev = phy_0;
>  }
> 
>  /**
> @@ -3053,7 +3100,7 @@ static int ice_fill_quad_msg_e82x(struct ice_hw
> *hw,
>  	if (quad >= ICE_GET_QUAD_NUM(hw->ptp.num_lports))
>  		return -EINVAL;
> 
> -	msg->dest_dev = rmn_0;
> +	msg->dest_dev = phy_0;
> 
>  	if (!(quad % ICE_GET_QUAD_NUM(hw->ptp.ports_per_phy)))
>  		addr = Q_0_BASE + offset;
> @@ -4772,7 +4819,7 @@ static int ice_read_phy_reg_e810(struct ice_hw
> *hw, u32 addr, u32 *val)
>  	msg.msg_addr_low = lower_16_bits(addr);
>  	msg.msg_addr_high = upper_16_bits(addr);
>  	msg.opcode = ice_sbq_msg_rd;
> -	msg.dest_dev = rmn_0;
> +	msg.dest_dev = phy_0;
> 
>  	err = ice_sbq_rw_reg(hw, &msg, ICE_AQ_FLAG_RD);
>  	if (err) {
> @@ -4802,7 +4849,7 @@ static int ice_write_phy_reg_e810(struct ice_hw
> *hw, u32 addr, u32 val)
>  	msg.msg_addr_low = lower_16_bits(addr);
>  	msg.msg_addr_high = upper_16_bits(addr);
>  	msg.opcode = ice_sbq_msg_wr;
> -	msg.dest_dev = rmn_0;
> +	msg.dest_dev = phy_0;
>  	msg.data = val;
> 
>  	err = ice_sbq_rw_reg(hw, &msg, ICE_AQ_FLAG_RD);
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> index 35141198f261..d913ed00c730 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
> @@ -65,14 +65,14 @@ enum ice_eth56g_link_spd {
> 
>  /**
>   * struct ice_phy_reg_info_eth56g - ETH56G PHY register parameters
> - * @base: base address for each PHY block
> + * @base_addr: base address for each PHY block
>   * @step: step between PHY lanes
>   *
>   * Characteristic information for the various PHY register parameters in the
>   * ETH56G devices
>   */
>  struct ice_phy_reg_info_eth56g {
> -	u32 base[NUM_ETH56G_PHY_RES];
> +	u32 base_addr;
>  	u32 step;
>  };
> 
> @@ -773,37 +773,20 @@ static inline u64 ice_get_base_incval(struct ice_hw
> *hw)
>  #define PHY_MAC_XIF_TS_SFD_ENA_M	ICE_M(0x1, 20)
>  #define PHY_MAC_XIF_GMII_TS_SEL_M	ICE_M(0x1, 21)
> 
> -/* GPCS config register */
> -#define PHY_GPCS_CONFIG_REG0		0x268
> -#define PHY_GPCS_CONFIG_REG0_TX_THR_M	ICE_M(0xF, 24)
> -#define PHY_GPCS_BITSLIP		0x5C
> -
>  #define PHY_TS_INT_CONFIG_THRESHOLD_M	ICE_M(0x3F, 0)
>  #define PHY_TS_INT_CONFIG_ENA_M		BIT(6)
> 
> -/* 1-step PTP config */
> -#define PHY_PTP_1STEP_CONFIG		0x270
> -#define PHY_PTP_1STEP_T1S_UP64_M	ICE_M(0xF, 4)
> -#define PHY_PTP_1STEP_T1S_DELTA_M	ICE_M(0xF, 8)
> -#define PHY_PTP_1STEP_PEER_DELAY(_port)	(0x274 + 4 * (_port))
> -#define PHY_PTP_1STEP_PD_ADD_PD_M	ICE_M(0x1, 0)
> -#define PHY_PTP_1STEP_PD_DELAY_M	ICE_M(0x3fffffff, 1)
> -#define PHY_PTP_1STEP_PD_DLY_V_M	ICE_M(0x1, 31)
> -
>  /* Macros to derive offsets for TimeStampLow and TimeStampHigh */
>  #define PHY_TSTAMP_L(x) (((x) * 8) + 0)
>  #define PHY_TSTAMP_U(x) (((x) * 8) + 4)
> 
> -#define PHY_REG_REVISION		0x85000
> -
>  #define PHY_REG_DESKEW_0		0x94
>  #define PHY_REG_DESKEW_0_RLEVEL		GENMASK(6, 0)
>  #define PHY_REG_DESKEW_0_RLEVEL_FRAC	GENMASK(9, 7)
>  #define PHY_REG_DESKEW_0_RLEVEL_FRAC_W	3
>  #define PHY_REG_DESKEW_0_VALID		GENMASK(10, 10)
> 
> -#define PHY_REG_GPCS_BITSLIP		0x5C
> -#define PHY_REG_SD_BIT_SLIP(_port_offset)	(0x29C + 4 * (_port_offset))
> +#define PHY_REG_REVISION		0x85000
>  #define PHY_REVISION_ETH56G		0x10200
>  #define PHY_VENDOR_TXLANE_THRESH	0x2000C
> 
> @@ -822,7 +805,21 @@ static inline u64 ice_get_base_incval(struct ice_hw
> *hw)
>  #define PHY_MAC_BLOCKTIME		0x50
>  #define PHY_MAC_MARKERTIME		0x54
>  #define PHY_MAC_TX_OFFSET		0x58
> +#define PHY_GPCS_BITSLIP		0x5C
> 
>  #define PHY_PTP_INT_STATUS		0x7FD140
> 
> +/* ETH56G registers shared per quad */
> +/* GPCS config register */
> +#define PHY_GPCS_CONFIG_REG0		0x268
> +#define PHY_GPCS_CONFIG_REG0_TX_THR_M	GENMASK(27, 24)
> +/* 1-step PTP config */
> +#define PHY_PTP_1STEP_CONFIG		0x270
> +#define PHY_PTP_1STEP_T1S_UP64_M	GENMASK(7, 4)
> +#define PHY_PTP_1STEP_T1S_DELTA_M	GENMASK(11, 8)
> +#define PHY_PTP_1STEP_PEER_DELAY(_quad_lane)	(0x274 + 4 *
> (_quad_lane))
> +#define PHY_PTP_1STEP_PD_ADD_PD_M	BIT(0)
> +#define PHY_PTP_1STEP_PD_DELAY_M	GENMASK(30, 1)
> +#define PHY_PTP_1STEP_PD_DLY_V_M	BIT(31)
> +#define PHY_REG_SD_BIT_SLIP(_quad_lane)	(0x29C + 4 * (_quad_lane))
>  #endif /* _ICE_PTP_HW_H_ */
> diff --git a/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
> b/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
> index 3b0054faf70c..f362607ece35 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
> +++ b/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
> @@ -47,12 +47,9 @@ struct ice_sbq_evt_desc {
>  };
> 
>  enum ice_sbq_msg_dev {
> -	eth56g_phy_0	= 0x02,
> -	rmn_0		= 0x02,
> -	rmn_1		= 0x03,
> -	rmn_2		= 0x04,
> +	phy_0		= 0x02,
>  	cgu		= 0x06,
> -	eth56g_phy_1	= 0x0D,
> +	phy_0_peer	= 0x0D,
>  };
> 
>  enum ice_sbq_msg_opcode {
> diff --git a/drivers/net/ethernet/intel/ice/ice_type.h
> b/drivers/net/ethernet/intel/ice/ice_type.h
> index 45768796691f..479227bdff75 100644
> --- a/drivers/net/ethernet/intel/ice/ice_type.h
> +++ b/drivers/net/ethernet/intel/ice/ice_type.h
> @@ -850,7 +850,6 @@ struct ice_mbx_data {
> 
>  struct ice_eth56g_params {
>  	u8 num_phys;
> -	u8 phy_addr[2];
>  	bool onestep_ena;
>  	bool sfd_ena;
>  	u32 peer_delay;
> --
> 2.46.2

[Intel-wired-lan] [PATCH v2 iwl-net 3/4] ice: Fix ETH56G FC-FEC Rx offset value

[Karol Kolacinski]

Fix ETH56G FC-FEC incorrect Rx offset value by changing it from -255.96
to -469.26 ns.

Those values are derived from HW spec and reflect internal delays.
Hex value is a fixed point representation in Q23.9 format.

Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_ptp_consts.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_consts.h b/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
index e63f2a36eabf..339b9f59ddde 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_consts.h
@@ -86,7 +86,7 @@ struct ice_eth56g_mac_reg_cfg eth56g_mac_cfg[NUM_ICE_ETH56G_LNK_SPD] = {
 		.rx_offset = {
 			.serdes = 0xffffeb27, /* -10.42424 */
 			.no_fec = 0xffffcccd, /* -25.6 */
-			.fc = 0xfffe0014, /* -255.96 */
+			.fc = 0xfffc557b, /* -469.26 */
 			.sfd = 0x4a4, /* 2.32 */
 			.bs_ds = 0x32 /* 0.0969697 */
 		}
-- 
2.46.2

[Intel-wired-lan] [PATCH v2 iwl-net 4/4] ice: Add correct PHY lane assignment

[Karol Kolacinski]

Driver always naively assumes, that for PTP purposes, PHY lane to
configure is corresponding to PF ID.

This is not true for some port configurations, e.g.:
- 2x50G per quad, where lanes used are 0 and 2 on each quad, but PF IDs
  are 0 and 1
- 100G per quad on 2 quads, where lanes used are 0 and 4, but PF IDs are
  0 and 1

Use correct PHY lane assignment by getting and parsing port options.
This is read from the NVM by the FW and provided to the driver with
the indication of active port split.

Remove ice_is_muxed_topo(), which is no longer needed.

Fixes: 4409ea1726cb ("ice: Adjust PTP init for 2x50G E825C devices")
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Arkadiusz Kubalewski <Arkadiusz.kubalewski@intel.com>
Signed-off-by: Karol Kolacinski <karol.kolacinski@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_common.c | 40 +++++++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_common.h |  1 +
 drivers/net/ethernet/intel/ice/ice_main.c   |  6 ++--
 drivers/net/ethernet/intel/ice/ice_ptp.c    | 23 +++++-------
 drivers/net/ethernet/intel/ice/ice_ptp.h    |  4 +--
 drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 22 ------------
 drivers/net/ethernet/intel/ice/ice_type.h   |  1 -
 7 files changed, 55 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 0eb7f828ed3a..618259f8abdc 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -4065,6 +4065,46 @@ ice_aq_set_port_option(struct ice_hw *hw, u8 lport, u8 lport_valid,
 	return ice_aq_send_cmd(hw, &desc, NULL, 0, NULL);
 }
 
+/**
+ * ice_get_phy_lane_number - Get PHY lane number for current adapter
+ * @hw: pointer to the hw struct
+ *
+ * Return: PHY lane number on success, negative error code otherwise.
+ */
+int ice_get_phy_lane_number(struct ice_hw *hw)
+{
+	struct ice_aqc_get_port_options_elem *options __free(kfree);
+	unsigned int lport = 0;
+	unsigned int lane;
+	int err;
+
+	options = kcalloc(ICE_AQC_PORT_OPT_MAX, sizeof(*options), GFP_KERNEL);
+	if (!options)
+		return -ENOMEM;
+
+	for (lane = 0; lane < ICE_MAX_PORT_PER_PCI_DEV; lane++) {
+		u8 options_count = ICE_AQC_PORT_OPT_MAX;
+		bool active_valid, pending_valid;
+		u8 active_idx, pending_idx;
+
+		err = ice_aq_get_port_options(hw, options, &options_count, lane,
+					      true, &active_idx, &active_valid,
+					      &pending_idx, &pending_valid);
+		if (err)
+			return err;
+
+		if (!active_valid)
+			continue;
+
+		if (hw->pf_id == lport)
+			return lane;
+		lport++;
+	}
+
+	/* PHY lane not found */
+	return -ENXIO;
+}
+
 /**
  * ice_aq_sff_eeprom
  * @hw: pointer to the HW struct
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index 27208a60cece..fe6f88cfd948 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -193,6 +193,7 @@ ice_aq_get_port_options(struct ice_hw *hw,
 int
 ice_aq_set_port_option(struct ice_hw *hw, u8 lport, u8 lport_valid,
 		       u8 new_option);
+int ice_get_phy_lane_number(struct ice_hw *hw);
 int
 ice_aq_sff_eeprom(struct ice_hw *hw, u16 lport, u8 bus_addr,
 		  u16 mem_addr, u8 page, u8 set_page, u8 *data, u8 length,
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index b1e7727b8677..72ef1b15b100 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1144,7 +1144,7 @@ ice_link_event(struct ice_pf *pf, struct ice_port_info *pi, bool link_up,
 	if (link_up == old_link && link_speed == old_link_speed)
 		return 0;
 
-	ice_ptp_link_change(pf, pf->hw.pf_id, link_up);
+	ice_ptp_link_change(pf, link_up);
 
 	if (ice_is_dcb_active(pf)) {
 		if (test_bit(ICE_FLAG_DCB_ENA, pf->flags))
@@ -6742,7 +6742,7 @@ static int ice_up_complete(struct ice_vsi *vsi)
 		ice_print_link_msg(vsi, true);
 		netif_tx_start_all_queues(vsi->netdev);
 		netif_carrier_on(vsi->netdev);
-		ice_ptp_link_change(pf, pf->hw.pf_id, true);
+		ice_ptp_link_change(pf, true);
 	}
 
 	/* Perform an initial read of the statistics registers now to
@@ -7212,7 +7212,7 @@ int ice_down(struct ice_vsi *vsi)
 
 	if (vsi->netdev) {
 		vlan_err = ice_vsi_del_vlan_zero(vsi);
-		ice_ptp_link_change(vsi->back, vsi->back->hw.pf_id, false);
+		ice_ptp_link_change(vsi->back, false);
 		netif_carrier_off(vsi->netdev);
 		netif_tx_disable(vsi->netdev);
 	}
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index ef2e858f49bb..da7b57684d32 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1454,10 +1454,9 @@ ice_ptp_port_phy_restart(struct ice_ptp_port *ptp_port)
 /**
  * ice_ptp_link_change - Reconfigure PTP after link status change
  * @pf: Board private structure
- * @port: Port for which the PHY start is set
  * @linkup: Link is up or down
  */
-void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup)
+void ice_ptp_link_change(struct ice_pf *pf, bool linkup)
 {
 	struct ice_ptp_port *ptp_port;
 	struct ice_hw *hw = &pf->hw;
@@ -1465,14 +1464,7 @@ void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup)
 	if (pf->ptp.state != ICE_PTP_READY)
 		return;
 
-	if (WARN_ON_ONCE(port >= hw->ptp.num_lports))
-		return;
-
 	ptp_port = &pf->ptp.port;
-	if (ice_is_e825c(hw) && hw->ptp.is_2x50g_muxed_topo)
-		port *= 2;
-	if (WARN_ON_ONCE(ptp_port->port_num != port))
-		return;
 
 	/* Update cached link status for this port immediately */
 	ptp_port->link_up = linkup;
@@ -3339,10 +3331,17 @@ void ice_ptp_init(struct ice_pf *pf)
 {
 	struct ice_ptp *ptp = &pf->ptp;
 	struct ice_hw *hw = &pf->hw;
-	int err;
+	int lane_num, err;
 
 	ptp->state = ICE_PTP_INITIALIZING;
 
+	lane_num = ice_get_phy_lane_number(hw);
+	if (lane_num < 0) {
+		err = lane_num;
+		goto err;
+	}
+
+	ptp->port.port_num = (u8)lane_num;
 	ice_ptp_init_hw(hw);
 
 	ice_ptp_init_tx_interrupt_mode(pf);
@@ -3356,10 +3355,6 @@ void ice_ptp_init(struct ice_pf *pf)
 			goto err;
 	}
 
-	ptp->port.port_num = hw->pf_id;
-	if (ice_is_e825c(hw) && hw->ptp.is_2x50g_muxed_topo)
-		ptp->port.port_num = hw->pf_id * 2;
-
 	err = ice_ptp_init_port(pf, &ptp->port);
 	if (err)
 		goto err;
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index 2db2257a0fb2..44a05c8d2113 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -331,7 +331,7 @@ void ice_ptp_prepare_for_reset(struct ice_pf *pf,
 			       enum ice_reset_req reset_type);
 void ice_ptp_init(struct ice_pf *pf);
 void ice_ptp_release(struct ice_pf *pf);
-void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup);
+void ice_ptp_link_change(struct ice_pf *pf, bool linkup);
 #else /* IS_ENABLED(CONFIG_PTP_1588_CLOCK) */
 static inline int ice_ptp_set_ts_config(struct ice_pf *pf, struct ifreq *ifr)
 {
@@ -379,7 +379,7 @@ static inline void ice_ptp_prepare_for_reset(struct ice_pf *pf,
 }
 static inline void ice_ptp_init(struct ice_pf *pf) { }
 static inline void ice_ptp_release(struct ice_pf *pf) { }
-static inline void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool linkup)
+static inline void ice_ptp_link_change(struct ice_pf *pf, bool linkup)
 {
 }
 
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index f28d0357c88f..3737727231b1 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -2699,26 +2699,6 @@ static int ice_get_phy_tx_tstamp_ready_eth56g(struct ice_hw *hw, u8 port,
 	return 0;
 }
 
-/**
- * ice_is_muxed_topo - detect breakout 2x50G topology for E825C
- * @hw: pointer to the HW struct
- *
- * Return: true if it's 2x50 breakout topology, false otherwise
- */
-static bool ice_is_muxed_topo(struct ice_hw *hw)
-{
-	u8 link_topo;
-	bool mux;
-	u32 val;
-
-	val = rd32(hw, GLGEN_SWITCH_MODE_CONFIG);
-	mux = FIELD_GET(GLGEN_SWITCH_MODE_CONFIG_25X4_QUAD_M, val);
-	val = rd32(hw, GLGEN_MAC_LINK_TOPO);
-	link_topo = FIELD_GET(GLGEN_MAC_LINK_TOPO_LINK_TOPO_M, val);
-
-	return (mux && link_topo == ICE_LINK_TOPO_UP_TO_2_LINKS);
-}
-
 /**
  * ice_ptp_init_phy_e825 - initialize PHY parameters
  * @hw: pointer to the HW struct
@@ -2743,8 +2723,6 @@ static void ice_ptp_init_phy_e825(struct ice_hw *hw)
 	err = ice_read_phy_eth56g(hw, hw->pf_id, PHY_REG_REVISION, &phy_rev);
 	if (err || phy_rev != PHY_REVISION_ETH56G)
 		ptp->phy_model = ICE_PHY_UNSUP;
-
-	ptp->is_2x50g_muxed_topo = ice_is_muxed_topo(hw);
 }
 
 /* E822 family functions
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index 479227bdff75..609f31e0dfde 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -880,7 +880,6 @@ struct ice_ptp_hw {
 	union ice_phy_params phy;
 	u8 num_lports;
 	u8 ports_per_phy;
-	bool is_2x50g_muxed_topo;
 };
 
 /* Port hardware description */
-- 
2.46.2

Re: [Intel-wired-lan] [PATCH v2 iwl-net 4/4] ice: Add correct PHY lane assignment

[Jacob Keller]

On 10/10/2024 7:21 AM, Karol Kolacinski wrote:

>  /**
>   * ice_ptp_init_phy_e825 - initialize PHY parameters
>   * @hw: pointer to the HW struct
> @@ -2743,8 +2723,6 @@ static void ice_ptp_init_phy_e825(struct ice_hw *hw)
>  	err = ice_read_phy_eth56g(hw, hw->pf_id, PHY_REG_REVISION, &phy_rev);
>  	if (err || phy_rev != PHY_REVISION_ETH56G)
>  		ptp->phy_model = ICE_PHY_UNSUP;
> -
> -	ptp->is_2x50g_muxed_topo = ice_is_muxed_topo(hw);
>  }
>  
Once we remove ptp->phy_model, this check for the PHY revision seems
like it is meaningless?

Thanks,
Jake

Re: [Intel-wired-lan] [PATCH v2 iwl-net 4/4] ice: Add correct PHY lane assignment

[Nitka, Grzegorz]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: Thursday, October 17, 2024 1:29 AM
> To: Kolacinski, Karol <karol.kolacinski@intel.com>; intel-wired-
> lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Kubalewski, Arkadiusz
> <arkadiusz.kubalewski@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH v2 iwl-net 4/4] ice: Add correct PHY
> lane assignment
> 
> 
> 
> On 10/10/2024 7:21 AM, Karol Kolacinski wrote:
> 
> >  /**
> >   * ice_ptp_init_phy_e825 - initialize PHY parameters
> >   * @hw: pointer to the HW struct
> > @@ -2743,8 +2723,6 @@ static void ice_ptp_init_phy_e825(struct ice_hw
> *hw)
> >  	err = ice_read_phy_eth56g(hw, hw->pf_id, PHY_REG_REVISION,
> &phy_rev);
> >  	if (err || phy_rev != PHY_REVISION_ETH56G)
> >  		ptp->phy_model = ICE_PHY_UNSUP;
> > -
> > -	ptp->is_2x50g_muxed_topo = ice_is_muxed_topo(hw);
> >  }
> >
> Once we remove ptp->phy_model, this check for the PHY revision seems
> like it is meaningless?
> 
> Thanks,
> Jake

Agree, to be removed in v3.

Re: [Intel-wired-lan] [PATCH v2 iwl-net 0/4] Fix E825 initialization

[Jacob Keller]

On 10/10/2024 7:21 AM, Karol Kolacinski wrote:
> E825 products have incorrect initialization procedure, which may lead to
> initialization failures and register values.
> 
> Fix E825 products initialization by adding correct sync delay, checking
> the PHY revision only for current PHY and adding proper destination
> device when reading port/quad.
> 
> In addition, E825 uses PF ID for indexing per PF registers and as
> a primary PHY lane number, which is incorrect.
> 
> Karol Kolacinski (4):
>   ice: Fix E825 initialization
>   ice: Fix quad registers read on E825
>   ice: Fix ETH56G FC-FEC Rx offset value
>   ice: Add correct PHY lane assignment
> 

I applied these to net-queue, but they don't apply cleanly to next
queue, due to conflicts with the PHY model changes and some other issues.

I also had some questions on one of the patches.

Thanks,
Jake

>  drivers/net/ethernet/intel/ice/ice_common.c   |  42 ++-
>  drivers/net/ethernet/intel/ice/ice_common.h   |   1 +
>  drivers/net/ethernet/intel/ice/ice_main.c     |   6 +-
>  drivers/net/ethernet/intel/ice/ice_ptp.c      |  23 +-
>  drivers/net/ethernet/intel/ice/ice_ptp.h      |   4 +-
>  .../net/ethernet/intel/ice/ice_ptp_consts.h   |  77 +----
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.c   | 281 ++++++++++--------
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.h   |  40 ++-
>  drivers/net/ethernet/intel/ice/ice_sbq_cmd.h  |   7 +-
>  drivers/net/ethernet/intel/ice/ice_type.h     |   2 -
>  10 files changed, 243 insertions(+), 240 deletions(-)
> 
> V1 -> V2: Removed net-next hunks from "ice: Fix E825 initialization",
>           whole "ice: Remove unnecessary offset calculation for PF
>           scoped registers" patch and fixed kdoc in "ice: Fix quad
>           registers read on E825"
> 
> base-commit: ab8024f0207eec0f376591c958b5bd30a03ae370

Re: [Intel-wired-lan] [PATCH v2 iwl-net 0/4] Fix E825 initialization

[Nitka, Grzegorz]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Jacob Keller
> Sent: Thursday, October 17, 2024 1:28 AM
> To: Kolacinski, Karol <karol.kolacinski@intel.com>; intel-wired-
> lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH v2 iwl-net 0/4] Fix E825 initialization
> 
> 
> 
> On 10/10/2024 7:21 AM, Karol Kolacinski wrote:
> > E825 products have incorrect initialization procedure, which may lead to
> > initialization failures and register values.
> >
> > Fix E825 products initialization by adding correct sync delay, checking
> > the PHY revision only for current PHY and adding proper destination
> > device when reading port/quad.
> >
> > In addition, E825 uses PF ID for indexing per PF registers and as
> > a primary PHY lane number, which is incorrect.
> >
> > Karol Kolacinski (4):
> >   ice: Fix E825 initialization
> >   ice: Fix quad registers read on E825
> >   ice: Fix ETH56G FC-FEC Rx offset value
> >   ice: Add correct PHY lane assignment
> >
> 
> I applied these to net-queue, but they don't apply cleanly to next
> queue, due to conflicts with the PHY model changes and some other issues.
> 
> I also had some questions on one of the patches.
> 

Jake, please ignore my previous comments about rebase.
Like you noticed, more rework is required for net-next.
I have a patchset which addresses that.
In the meantime, let's resolve all the comments which applies to net-queue.

> Thanks,
> Jake
> 
> >  drivers/net/ethernet/intel/ice/ice_common.c   |  42 ++-
> >  drivers/net/ethernet/intel/ice/ice_common.h   |   1 +
> >  drivers/net/ethernet/intel/ice/ice_main.c     |   6 +-
> >  drivers/net/ethernet/intel/ice/ice_ptp.c      |  23 +-
> >  drivers/net/ethernet/intel/ice/ice_ptp.h      |   4 +-
> >  .../net/ethernet/intel/ice/ice_ptp_consts.h   |  77 +----
> >  drivers/net/ethernet/intel/ice/ice_ptp_hw.c   | 281 ++++++++++--------
> >  drivers/net/ethernet/intel/ice/ice_ptp_hw.h   |  40 ++-
> >  drivers/net/ethernet/intel/ice/ice_sbq_cmd.h  |   7 +-
> >  drivers/net/ethernet/intel/ice/ice_type.h     |   2 -
> >  10 files changed, 243 insertions(+), 240 deletions(-)
> >
> > V1 -> V2: Removed net-next hunks from "ice: Fix E825 initialization",
> >           whole "ice: Remove unnecessary offset calculation for PF
> >           scoped registers" patch and fixed kdoc in "ice: Fix quad
> >           registers read on E825"
> >
> > base-commit: ab8024f0207eec0f376591c958b5bd30a03ae370