[PATCH net-next v23 0/3] net: phy: Add driver for Motorcomm Quad 2.5GbE phy

[PATCH net-next v23 0/3] net: phy: Add driver for Motorcomm Quad 2.5GbE phy

[Kyle Switch]

This patchset mainly implements the phy8824 driver. The phy8824 is
an Ethernet PHY that provides one 10G SerDes output to four 2.5G ports.
The driver mainly covers two application scenarios:
 1. external PHY8824
 2. PHY8824 embedded in a switch.
The patchset mainly consists of three parts:
 1. the DTS for the phy8824
 2. a generic template testmode configuration api.
 3. the phy8824 functional functions.

changes in v23:
1) Remove meaningless comments in driver.
2) Using #define to replace magic numbers during PHY8824 init.

changes in v22:
1) Using motorcomm,package-mode to replace phy-mode in DTS.
2) The corresponding parsing has also been updated, using
   "motorcomm,package-mode".

changes in v21:
patch 1: 1) Add maintainer for motorcomm,yt8824.yaml.
    2) Using 10g-qxgmii instead of usxgmii.
patch 3: 1) Change the usxgmii space to serdes space.
         2) Add lock operation during phy-package init in probe()
         3) Add check for whether auto-negotiation is enabled
         when updating the link status in read_status().

changes in v20:
patch 1: Add phy-mode description in properties in motorcomm,yt8824.yaml

changes in v19:
patch 2: Replace the int type with the u16 type for test_mode.
patch 3: 1) Merge yt8824_utp_normal_test_mode_paged() and
         yt8824_utp_invalid_test_mode_paged() into 
         yt8824_utp_set_template_test_mode().
  2) Add a fatal error return when phy-mode is missing in
         the DTS.

changes in v18:
1. Split the template_testmode() into a small patch 
   and optimize the implementation.
2. Optimize the definition and initialization of 
   interface_mode by adopting the existing approach.

changes in v17:
patch 1: Remove redundant description.
         Use phy-mode instead of motorcomm,interface to describe the
         interface.
         Add the required checks.
patch 2: Use phy-mode to get interface mode.
         Need return after init configuration is done to avoid falling
         into the error path.

changes in v16:
1. Add DTS documentation.
2. Fix redundant initialization.

changes in v14:
1. Refactor the error return path to preserve and return the initial error,
   rather than letting later errors mask it.

changes in v13:
1. Fix the lock release issue on the error path to prevent potential
   deadlock or resource leak.
2. Add a func yt8824_restore_defaults() to restore configuration on
   the error path,ensuring the hardware/device is left in a known
   good state upon failure.

changes in v12:
1. Refactor the yt8824_read_status() function to ensure proper state
   synchronization between hardware registers and the phy_device structure.

changes in v11:
1. Clean build_clang warning.

changes in v10:
1. Within the interface that swaps to the USXGMII reg space,the lock
   include shared_lock and mdio mutex must remain held throughout 
   the entire operation and should only be released after all steps
   have completed.
2. Improve the template interface in phy-c45.c
3. Fix the handling of some failure paths.

changes in v9:
1. Add shared_lock mutex prevents UTPs from interfering with each other 
   due to swapping reg space.

change in v8:
1. Clean up format warning.
2. Fix exception handling code logic based on Sashiko/Gemini.
3. Remove interrupt/handle function

changes in v7:
1. Refactor the using of mdio lock
  In all cases of swapping to the USXGMII side interface, lock the MDIO bus
  at the beginning, switch to the UTP address space after the operation is
  completed, and then release the lock.The purpose of doing this is to
  ensure that it will not affect other UTPs operating in the UTP address
  space.
2. Rename YT8824_RSSR_FIBER_SPACE to YT8824_RSSR_USXGMII_SPACE.

changes in v6:
1. Add  test mode helper in phy-c45.c
2. Refactor the using of swapping paged for utp and fiber

changes in v5:
1. Fix diffs issue which caused by unexpected whitespace.
2. Fix "exceeding 80 columns" warning.

changes in v4:
1. Remove motorcomm,yt8xxx.yaml, will update in other patch thread
2. Fix locking issue. Since every interface requires switching of space, 
   the bus is already locked during the space switching process in
   phy_select_page(), other operations within the interface cannot 
   be locked again before unlock in phy_restore_page().
3. Fix warning log identified during the inspection process using
   checkpatch.pl.
4. Fix the way of using common api in phy_package.c


changes in v3:
1. Using common apis defined in phy_package.c to handle shared top
extend register space.
2. Add dts demo in motorcomm,yt8xxx.yaml.
3. Fix unnecessary redundant judgments.
4. Fix BMCR registers operation using magic number.
5. Rename function based on its approximate functionality.

changes in v2:
1. Remove duplicate code and replace it with existing api.

Kyle Switch (3):
  dt-bindings: net: Document Motorcomm YT8824 PHY package
  net: phy: Add support for Template Control register for PMA
  net: phy: Add driver for Motorcomm Quad 2.5GbE phy

 .../bindings/net/motorcomm,yt8824.yaml        |   78 +
 MAINTAINERS                                   |    2 +
 drivers/net/phy/Kconfig                       |    3 +-
 drivers/net/phy/motorcomm.c                   | 1887 ++++++++++++++++-
 drivers/net/phy/phy-c45.c                     |   23 +
 include/linux/phy.h                           |    1 +
 include/uapi/linux/mdio.h                     |   12 +
 7 files changed, 2003 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml

-- 
2.25.1

[PATCH net-next v23 1/3] dt-bindings: net: Document Motorcomm YT8824 PHY package

[Kyle Switch]

Motorcomm YT8824 Ethernet PHY is PHY package of 4 PHY-s.

Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 .../bindings/net/motorcomm,yt8824.yaml        | 78 +++++++++++++++++++
 MAINTAINERS                                   |  2 +
 2 files changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml

diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml
new file mode 100644
index 000000000000..4c67f8afcf27
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/motorcomm,yt8824.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MotorComm YT8824 Ethernet PHY
+
+maintainers:
+  - Kyle Switch <kyle.switch@motor-comm.com>
+
+description:
+  Motorcomm YT8824 Ethernet PHY is a PHY package of 4 PHYs.
+
+$ref: ethernet-phy-package.yaml#
+
+properties:
+  compatible:
+    enum:
+      - motorcomm,yt8824-package
+
+  motorcomm,package-mode:
+    description: |
+      PHY package can be configured in 2 modes:
+      - internal: YT8824 is embedded in the switch, using
+        an internal interface to the MAC.
+      - 10g-qxgmii: YT8824 is a standalone external PHY,
+        connected via 10G QXGMII.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [ internal, 10g-qxgmii ]
+
+  reg:
+    description:
+      The absolute MDIO address of the YT8824 shared top extend
+      register block.
+      For internal mode, this is fixed at 9;
+      For external mode, it is the base MDIO address of the four
+      member PHYs plus 4, where base is typically 0x0.
+      Note that this is NOT the reg of the first member PHY.
+
+required:
+  - compatible
+  - motorcomm,package-mode
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ethernet-phy-package@9 {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            compatible = "motorcomm,yt8824-package";
+            reg = <9>;
+
+            motorcomm,package-mode = "internal";
+
+            ethernet-phy@4 {
+                reg = <4>;
+            };
+
+            ethernet-phy@5 {
+                reg = <5>;
+            };
+
+            ethernet-phy@6 {
+                reg = <6>;
+            };
+
+            ethernet-phy@7 {
+                reg = <7>;
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 51873349ba91..a322b49fcfc6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18413,8 +18413,10 @@ F:	drivers/net/ethernet/stmicro/stmmac/dwmac-motorcomm.c
 
 MOTORCOMM PHY DRIVER
 M:	Frank <Frank.Sae@motor-comm.com>
+M:	Kyle Switch <kyle.switch@motor-comm.com>
 L:	netdev@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml
 F:	Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
 F:	drivers/net/phy/motorcomm.c
 
-- 
2.25.1

[PATCH net-next v23 2/3] net: phy: Add support for Template Control register for PMA

[Kyle Switch]

Add support for the 10GBASE-T PMA Template Test Mode register
field, which allows selecting one of eight test modes (Normal and
TestMode1..TestMode7) used for PHY validation.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 drivers/net/phy/phy-c45.c | 23 +++++++++++++++++++++++
 include/linux/phy.h       |  1 +
 include/uapi/linux/mdio.h | 12 ++++++++++++
 3 files changed, 36 insertions(+)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 870920311f9a..c5f5753f7194 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -1408,6 +1408,29 @@ int genphy_c45_fast_retrain(struct phy_device *phydev, bool enable)
 }
 EXPORT_SYMBOL_GPL(genphy_c45_fast_retrain);
 
+/**
+ * genphy_c45_template_testmode - configure template testmode registers
+ * @phydev: target phy_device struct
+ * @test_mode: testmode includes Normal to Test mode 7
+ *
+ * Description: Set template testmode include Normal to Test mode 7
+ *
+ * Return: 0 on success, or a negative error code on failure (e.g. register
+ * read/write error).
+ */
+int genphy_c45_template_testmode(struct phy_device *phydev, u16 test_mode)
+{
+	u16 ctrl;
+
+	if (test_mode > MDIO_PMA_10GBT_TESTMODE_7)
+		return -EOPNOTSUPP;
+
+	ctrl = FIELD_PREP(MDIO_PMA_10GBT_TESTMODE_MASK, test_mode);
+	return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_10GBT_TESTMODE,
+			      MDIO_PMA_10GBT_TESTMODE_MASK, ctrl);
+}
+EXPORT_SYMBOL_GPL(genphy_c45_template_testmode);
+
 /**
  * genphy_c45_plca_get_cfg - get PLCA configuration from standard registers
  * @phydev: target phy_device struct
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7c5098a0dd6c..b9dee2655e6b 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -2357,6 +2357,7 @@ int genphy_c45_loopback(struct phy_device *phydev, bool enable, int speed);
 int genphy_c45_pma_resume(struct phy_device *phydev);
 int genphy_c45_pma_suspend(struct phy_device *phydev);
 int genphy_c45_fast_retrain(struct phy_device *phydev, bool enable);
+int genphy_c45_template_testmode(struct phy_device *phydev, u16 test_mode);
 int genphy_c45_plca_get_cfg(struct phy_device *phydev,
 			    struct phy_plca_cfg *plca_cfg);
 int genphy_c45_plca_set_cfg(struct phy_device *phydev,
diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
index 06f4bc3c20c7..353ca13c64fc 100644
--- a/include/uapi/linux/mdio.h
+++ b/include/uapi/linux/mdio.h
@@ -63,6 +63,7 @@
 /* Media-dependent registers. */
 #define MDIO_PMA_10GBT_SWAPPOL	130	/* 10GBASE-T pair swap & polarity */
 #define MDIO_PMA_10GBT_TXPWR	131	/* 10GBASE-T TX power control */
+#define MDIO_PMA_10GBT_TESTMODE 132 /* Test mode control */
 #define MDIO_PMA_10GBT_SNR	133	/* 10GBASE-T SNR margin, lane A.
 					 * Lanes B-D are numbered 134-136. */
 #define MDIO_PMA_10GBR_FSRT_CSR	147	/* 10GBASE-R fast retrain status and control */
@@ -320,6 +321,17 @@
 /* PMA 10GBASE-R Fast Retrain status and control register. */
 #define MDIO_PMA_10GBR_FSRT_ENABLE	0x0001	/* Fast retrain enable */
 
+/* PMA 10GBASE-T Template Test Mode Register*/
+#define MDIO_PMA_10GBT_TESTMODE_MASK GENMASK(15, 13) /* Template test mode */
+#define MDIO_PMA_10GBT_TESTMODE_NORMAL 0x0 /* Template Normal */
+#define MDIO_PMA_10GBT_TESTMODE_1 0x1 /* Template TestMode1 */
+#define MDIO_PMA_10GBT_TESTMODE_2 0x2 /* Template TestMode2 */
+#define MDIO_PMA_10GBT_TESTMODE_3 0x3 /* Template TestMode3 */
+#define MDIO_PMA_10GBT_TESTMODE_4 0x4 /* Template TestMode4 */
+#define MDIO_PMA_10GBT_TESTMODE_5 0x5 /* Template TestMode5 */
+#define MDIO_PMA_10GBT_TESTMODE_6 0x6 /* Template TestMode6 */
+#define MDIO_PMA_10GBT_TESTMODE_7 0x7 /* Template TestMode7 */
+
 /* PCS 10GBASE-R/-T status register 1. */
 #define MDIO_PCS_10GBRT_STAT1_BLKLK	0x0001	/* Block lock attained */
 
-- 
2.25.1

[PATCH net-next v23 3/3] net: phy: Add driver for Motorcomm Quad 2.5GbE phy

[Kyle Switch]

Add support for Motorcomm YT8824 quad-port 2.5G PHY to the existing
motorcomm driver, using the phy_package helpers for the shared top
extended register space.

Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 drivers/net/phy/Kconfig     |    3 +-
 drivers/net/phy/motorcomm.c | 1887 ++++++++++++++++++++++++++++++++++-
 2 files changed, 1887 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index d3835597e379..996d75afed44 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -361,9 +361,10 @@ config MICROSEMI_PHY
 
 config MOTORCOMM_PHY
 	tristate "Motorcomm PHYs"
+	select PHY_PACKAGE
 	help
 	  Enables support for Motorcomm network PHYs.
-	  Currently supports YT85xx Gigabit Ethernet PHYs.
+	  Currently supports YT85xx Gigabit Ethernet PHYs and YT8824 4 * 2.5G PHY.
 
 config NATIONAL_PHY
 	tristate "National Semiconductor PHYs"
diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 90a4f86f2758..dfa0612d713c 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -1,24 +1,31 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Motorcomm 8511/8521/8522/8531/8531S/8821 PHY driver.
+ * Motorcomm 8511/8521/8522/8531/8531S/8821/8824 PHY driver.
  *
  * Author: Peter Geis <pgwipeout@gmail.com>
  * Author: Frank <Frank.Sae@motor-comm.com>
+ * Author: Kyle <kyle.switch@motor-comm.com>
  */
 
 #include <linux/clk.h>
 #include <linux/etherdevice.h>
 #include <linux/kernel.h>
+#include <linux/mdio.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
 #include <linux/phy.h>
 #include <linux/property.h>
 
+#include "phylib.h"
+
 #define PHY_ID_YT8511		0x0000010a
 #define PHY_ID_YT8521		0x0000011a
 #define PHY_ID_YT8522		0x4f51e928
 #define PHY_ID_YT8531		0x4f51e91b
 #define PHY_ID_YT8531S		0x4f51e91a
 #define PHY_ID_YT8821		0x4f51ea19
+#define PHY_ID_YT8824		0x4f51e8b8
 /* YT8521/YT8531S/YT8821 Register Overview
  *	UTP Register space	|	FIBER Register space
  *  ------------------------------------------------------------
@@ -30,6 +37,18 @@
  *  ------------------------------------------------------------
  */
 
+/* YT8824 Register Overview
+ * UTP Register space	| SERDES Register space
+ *  ------------------------------------------------------------
+ * | UTP MII		| SERDES MII		|
+ * | UTP MMD		|			|
+ * | UTP Extended	| SERDES Extended	|
+ * | UTP Top Extended	| SERDES Top Extended	|
+ *  ------------------------------------------------------------
+ * |   Common Top Extended			|
+ *  ------------------------------------------------------------
+ */
+
 /* 0x10 ~ 0x15 , 0x1E and 0x1F are common MII registers of yt phy */
 
 /* Specific Function Control Register */
@@ -381,6 +400,226 @@
 #define YT8821_CHIP_MODE_AUTO_BX2500_SGMII	0
 #define YT8821_CHIP_MODE_FORCE_BX2500		1
 
+#define YT8824_RSSR_SPACE_MASK			BIT(0)
+#define YT8824_RSSR_SERDES_SPACE		(0x1)
+#define YT8824_RSSR_UTP_SPACE			(0x0)
+#define YT8824_SDS_CFG_MIN_PRE_MASK		GENMASK(3, 0)
+#define YT8824_SDS_EN_FILL_PRE			BIT(13)
+#define YT8824_SDS_TX_PRE_PADDING		(0x7)
+
+/* PHY8824 UTP init registers */
+#define YT8824_WRITE_PROTECT_CAL_REG		0x1
+#define YT8824_WRITE_PROTECT			BIT(0)
+#define YT8824_PLL_VCO_ATEMP_REG		0xa20e
+#define YT8824_PLL_VCO_SEL_CURRENT		GENMASK(7, 4)
+#define YT8824_PLL_VCO_SEL_CURRENT_B		0xb
+#define YT8824_PLL_PFD_CTRL_REG			0xa20a
+#define YT8824_PLL_VCO_ICOSTANT			GENMASK(9, 4)
+#define YT8824_PLL_VCO_ATEMP_1_REG		0xa20c
+#define YT8824_PLL_VCO_IPTAT			GENMASK(5, 0)
+#define YT8824_PLL_DAC_0_REG			0xa2b6
+#define YT8824_PLL_DAC_RST			BIT(10)
+#define YT8824_IF_CTRL_REG			0xa003
+#define YT8824_U0_U1_ENABEL_ODD_PREAMBLE	GENMASK(1, 0)
+#define YT8824_IDLE_CTRL_REG			0x3d0
+#define YT8824_IDLE_DETECT			BIT(9)
+#define YT8824_TRACE_GAIN_THR_REG		0x372
+#define YT8824_TRACE_LNG_GAIN_THR_2500		GENMASK(14, 8)
+#define YT8824_TRACE_MED_GAIN_THR_2500		GENMASK(6, 0)
+#define YT8824_PRM_LONG_REG			0x37C
+#define YT8824_PRM_LARGE_SLAVE_2500		GENMASK(10, 0)
+#define YT8824_PRM_SMALL_REG			0x388
+#define YT8824_PRM_SMALL_LNG_2500		GENMASK(10, 0)
+#define YT8824_FAST_RETARIN_REG			0x359
+#define YT8824_LINK_FAIL_SIG			GENMASK(13, 8)
+#define YT8824_EN_GATE_CTRL_REG			0xc
+#define YT8824_EN_GATE_GMII_CLK			BIT(0)
+#define YT8824_DAC_CFG_REG			0xa2fa
+#define YT8824_POWER_SAVIE			GENMASK(15, 0)
+#define YT8824_DAC_BIAS_CFG_REG			0x4e2
+#define YT8824_DAC_BIAS_CFG_2500		GENMASK(9, 5)
+#define YT8824_DAC_IMID_CFG_REG			0x47e
+#define YT8824_DAC_IMID_CH2			GENMASK(6, 0)
+#define YT8824_DAC_IMID_CH3			GENMASK(14, 8)
+#define YT8824_DAC_IMID_CFG1_REG		0x47f
+#define YT8824_DAC_IMID_CH0			GENMASK(6, 0)
+#define YT8824_DAC_IMID_CH1			GENMASK(14, 8)
+#define YT8824_DAC_IMSB_CFG_REG			0x480
+#define YT8824_DAC_IMSB_CH2			GENMASK(6, 0)
+#define YT8824_DAC_IMSB_CH3			GENMASK(14, 8)
+#define YT8824_DAC_IMSB_CFG1_REG		0x481
+#define YT8824_DAC_IMSB_CH0			GENMASK(6, 0)
+#define YT8824_DAC_IMSB_CH1			GENMASK(14, 8)
+#define YT8824_TRACE_LNG_GAIN_REG		0x336
+#define YT8824_TRACE_LNG_GAIN_THR		GENMASK(14, 8)
+#define YT8824_TRACE_MED_GAIN_REG		0x340
+#define YT8824_TRACE_MED_GAIN_THR		GENMASK(6, 0)
+#define YT8824_DAC_IMID_CH2_CH3_CFG_ORG_REG	0x46e
+#define YT8824_DAC_IMID_CH3_ORG			GENMASK(14, 8)
+#define YT8824_DAC_IMID_CH2_ORG			GENMASK(6, 0)
+#define YT8824_DAC_IMID_CH0_CH1_CFG_ORG_REG	0x46f
+#define YT8824_DAC_IMID_CH1_ORG			GENMASK(14, 8)
+#define YT8824_DAC_IMID_CH0_ORG			GENMASK(6, 0)
+#define YT8824_DAC_IMSB_CH2_CH3_CFG_ORG_REG	0x470
+#define YT8824_DAC_IMSB_CH3_ORG			GENMASK(14, 8)
+#define YT8824_DAC_IMSB_CH2_ORG			GENMASK(6, 0)
+#define YT8824_DAC_IMSB_CH0_CH1_CFG_ORG_REG	0x471
+#define YT8824_DAC_IMSB_CH1_ORG			GENMASK(14, 8)
+#define YT8824_DAC_IMSB_CH0_ORG			GENMASK(6, 0)
+#define YT8824_TRACE_LNG_GAIN_100_REG		0x030b
+#define YT8824_TRACE_LNG_GAIN_THR_100		GENMASK(14, 8)
+#define YT8824_TRACE_MED_GAIN_THR_100		GENMASK(6, 0)
+#define YT8824_CABLE_VSHT_REG			0x071f
+#define YT8824_CABLE_VSHT_TH_S			GENMASK(6, 0)
+#define YT8824_DAC_LPFIL_CH2_CH3_ADJ_REG	0x046b
+#define YT8824_DAC_LPFIL_ADJ_CH3_10		GENMASK(12, 8)
+#define YT8824_DAC_LPFIL_ADJ_CH2_10		GENMASK(4, 0)
+#define YT8824_DAC_LPFIL_CH0_CH1_ADJ_REG	0x046c
+#define YT8824_DAC_LPFIL_ADJ_CH1_10		GENMASK(12, 8)
+#define YT8824_DAC_LPFIL_ADJ_CH0_10		GENMASK(4, 0)
+#define YT8824_DAC_IMID_CFG_CH2_CH3_10_ORG	0x0466
+#define YT8824_DAC_IMID_CFG_CH3_10_ORG		GENMASK(14, 8)
+#define YT8824_DAC_IMID_CFG_CH2_10_ORG		GENMASK(6, 0)
+#define YT8824_DAC_IMID_CFG_CH0_CH1_10_ORG	0x0467
+#define YT8824_DAC_IMID_CFG_CH1_10_ORG		GENMASK(14, 8)
+#define YT8824_DAC_IMID_CFG_CH0_10_ORG		GENMASK(6, 0)
+#define YT8824_DAC_IMSB_CFG_CH2_CH3_10_ORG	0x0468
+#define YT8824_DAC_IMSB_CFG_CH3_10_ORG		GENMASK(14, 8)
+#define YT8824_DAC_IMSB_CFG_CH2_10_ORG		GENMASK(6, 0)
+#define YT8824_DAC_IMSB_CFG_CH0_CH1_10_ORG	0x0469
+#define YT8824_DAC_IMSB_CFG_CH1_10_ORG		GENMASK(14, 8)
+#define YT8824_DAC_IMSB_CFG_CH0_10_ORG		GENMASK(6, 0)
+#define YT8824_FFE_REG				0x034a
+#define YT8824_WAIT_TO_1000			GENMASK(15, 8)
+#define YT8824_FFE_SUM_THRES			GENMASK(2, 0)
+#define YT8824_EN_GATE_REG			0x00f8
+#define YT8824_EN_GATE_PHYDBG			BIT(10)
+#define YT8824_EN_GATE_BT1000			BIT(9)
+#define YT8824_CNT_REG				0x0059
+#define YT8824_CNT_ERR_AUTO			BIT(14)
+#define YT8824_PRM_LARGE_MASTER_REG		0x032c
+#define YT8824_PRM_LARGE_MASTER_1000		GENMASK(10, 0)
+#define YT8824_PRM_LARGE_SALVE_REG		0x032d
+#define YT8824_PRM_LARGE_SLAVE_1000		GENMASK(10, 0)
+#define YT8824_SIG_GONE_REG			0x032e
+#define YT8824_SIG_GONE_THR_1000		GENMASK(7, 0)
+#define YT8824_EN_NX_TRAIN_COARSE_REG		0x0322
+#define YT8824_EC_NX_TRAIN_COARSE_TO_1000	GENMASK(7, 0)
+#define YT8824_VGA_IN_LPF2_CAP_REG		0x04d3
+#define YT8824_VGA_IN_LPF2_CAP_OTHER		GENMASK(7, 4)
+#define YT8824_VGA_IN_LPF1_CAP_REG		0x04d2
+#define YT8824_VGA_IN_LPF1_CAP_OTHER		GENMASK(7, 4)
+#define YT8824_LDPC_LFER_FAIL_REG		0x00c8
+#define YT8824_LDPC_FAIL_TH			GENMASK(15, 8)
+#define YT8824_LFER_FAIL_TH			GENMASK(7, 0)
+#define YT8824_FR_REQ_REG			0x00be
+#define YT8824_FR_REQ_TH			GENMASK(4, 0)
+#define YT8824_GN_MU_CPARSE_REG			0x037a
+#define YT8824_PRM_SMALL_SHT_2500		GENMASK(10, 0)
+#define YT8824_DAC_AMP_ADJ_REG			0x0482
+#define YT8824_DAC_AMP_ADJ_CH3_PBO0_2500	GENMASK(15, 12)
+#define YT8824_DAC_AMP_ADJ_CH2_PBO0_2500	GENMASK(11, 8)
+#define YT8824_DAC_AMP_ADJ_CH1_PBO0_2500	GENMASK(7, 4)
+#define YT8824_DAC_AMP_ADJ_CH0_PBO0_2500	GENMASK(3, 0)
+#define YT8824_P0_DAC_BIAS_CAS_CH3_CH2_REG	0xa2d5
+#define YT8824_P0_DAC_BIAS_CAS_CFG_CH3		GENMASK(12, 8)
+#define YT8824_P0_DAC_BIAS_CAS_CFG_CH2		GENMASK(4, 0)
+#define YT8824_P0_DAC_BIAS_CAS_CH1_CH0_REG	0xa2d6
+#define YT8824_P0_DAC_BIAS_CAS_CFG_CH1		GENMASK(12, 8)
+#define YT8824_P0_DAC_BIAS_CAS_CFG_CH0		GENMASK(4, 0)
+#define YT8824_P1_DAC_BIAS_CAS_CH3_CH2_REG	0xa2d7
+#define YT8824_P1_DAC_BIAS_CAS_CFG_CH3		GENMASK(12, 8)
+#define YT8824_P1_DAC_BIAS_CAS_CFG_CH2		GENMASK(4, 0)
+#define YT8824_P1_DAC_BIAS_CAS_CH1_CH0_REG	0xa2d8
+#define YT8824_P1_DAC_BIAS_CAS_CFG_CH1		GENMASK(12, 8)
+#define YT8824_P1_DAC_BIAS_CAS_CFG_CH0		GENMASK(4, 0)
+#define YT8824_P01_PLL_EN_REG			0xa218
+#define YT8824_CSR_EFUSE_BYPASS_ANALOG_SECOND_RESET	BIT(6)
+#define YT8824_CSR_EFUSE_BYPASS_PLL_TXRX_RESET		BIT(5)
+#define YT8824_U0_CRS_RESET_REG			0xa01d
+#define YT8824_U0_CSR_RESETB_TX_CH0		BIT(3)
+#define YT8824_U0_CSR_RESETB_TX_CH1		BIT(2)
+#define YT8824_U0_CSR_RESETB_TX_CH2		BIT(1)
+#define YT8824_U0_CSR_RESETB_TX_CH3		BIT(0)
+#define YT8824_U1_CRS_RESET_REG			0xa01e
+#define YT8824_U1_CSR_RESETB_TX_CH0		BIT(3)
+#define YT8824_U1_CSR_RESETB_TX_CH1		BIT(2)
+#define YT8824_U1_CSR_RESETB_TX_CH2		BIT(1)
+#define YT8824_U1_CSR_RESETB_TX_CH3		BIT(0)
+/* External PHY8824 serdes init registers */
+#define YT8824_SERDES_PLL_CTRL1_REG		0xa13e
+#define YT8824_SERDES_PLL_CTRL2_REG		0xa13f
+#define YT8824_SERDES_PLL_CTRL3_REG		0xa140
+#define YT8824_WRITE_PROTECT_REG		0x4be
+#define YT8824_SERDES_REG_WR_PRT		BIT(2)
+#define YT8824_CSR_CTRL_REG			0x49f
+#define YT8824_CSR_PCS_GLB_RST_BYPASS		BIT(10)
+#define YT8824_CSR_SG_MODE_RST_BYPASS		BIT(9)
+#define YT8824_CSR_US_GLB_RESET_N		BIT(7)
+#define YT8824_CSR_PCS_BEF_TRAINING_DONE_RST_BYPASS	BIT(2)
+#define YT8824_CSR_PMA_CTRL_REG			0x4a9
+#define YT8824_CSR_WATCH_DOG_ALL_BYPASS		BIT(7)
+#define YT8824_CDR_CTRL_REG			0x406
+#define YT8824_CDR_MODE				BIT(11)
+#define YT8824_CSR_PLL_VCO_CTRL_REG		0x438
+#define YT8824_CSR_PLL_VCO_SEL_VCO2_CURRENT	GENMASK(15, 12)
+#define YT8824_CSR_PLL_VCO_ATEMP_R		GENMASK(2, 0)
+#define YT8824_CSR_PLL_DIVLDO_CTRL_REG		0x439
+#define YT8824_CSR_PLL_VCO_SEL_VCO2_RM_R	GENMASK(1, 0)
+#define YT8824_CSR_PLL_FBDIV_REG		0x043a
+#define YT8824_CSR_PLL_FBDIV_SYNC_SELB		BIT(1)
+#define YT8824_CSR_PLL_SPARE_CTRL_REG		0x042a
+#define YT8824_CSR_PLL_SPARE			GENMASK(15, 0)
+#define YT8824_CDR_MANUAL_REG			0x0491
+#define YT8824_VDAC_AMP_TH_LNG			GENMASK(6, 0)
+#define YT8824_TRAINING_CTRL_REG		0x0492
+#define YT8824_VDAC_AMP_TH_MED			GENMASK(14, 8)
+#define YT8824_VDAC_AMP_TH_SHT			GENMASK(7, 0)
+#define YT8824_EOM_CTRL_REG			0x0454
+#define YT8824_EOD_DURATION_SEL			GENMASK(11, 8)
+#define YT8824_EOD_VTH				GENMASK(6, 0)
+#define YT8824_PGA_GAIN_CTRL_REG		0x0497
+#define YT8824_CDR_OFFSET_ERR_WTH		GENMASK(14, 8)
+#define YT8824_PGA_GAIN_MAX			GENMASK(6, 4)
+#define YT8824_PGA_GAIN_INIT			GENMASK(2, 0)
+#define YT8824_EOD_CTRL_REG			0x04cd
+#define YT8824_EOD_ERR_CNT_TH			GENMASK(15, 0)
+#define YT8824_CSR_AFE_CTRL_REG			0x04af
+#define YT8824_CSR_AFE_VCM_CFG			GENMASK(10, 8)
+#define YT8824_CSR_AFE_MANUAL_CTRL_1P25G	BIT(4)
+#define YT8824_CSR_AFE_MANUAL_CTRL_3P125G	BIT(3)
+#define YT8824_CSR_AFE_MANUAL_CTRL_10P3125G	BIT(2)
+#define YT8824_DFE_CTRL_REG			0x048a
+#define YT8824_DFE_TAP_POLARITY			BIT(12)
+#define YT8824_DFE_IDAC_TUNE			GENMASK(11, 10)
+#define YT8824_CSR_CDR_CTRL_REG			0x0408
+#define YT8824_CSR_CDR_OFFSET_DATA		GENMASK(14, 8)
+#define YT8824_DFE_VDAC_CODE_REG		0x04d6
+#define YT8824_DFE_VDAC_CODE_TH			GENMASK(6, 0)
+#define YT8824_RX_SPARE_CTRL_REG		0x044f
+#define YT8824_RX_SPARE_REG			GENMASK(15, 0)
+#define YT8824_DFE_VDAC_CTRL_REG		0x048e
+#define YT8824_DFE_VDAC_TUNE			GENMASK(11, 10)
+#define YT8824_DFE_VDAC_SEL			GENMASK(9, 8)
+#define YT8824_CSR_TX_PRE_REG			0x000d
+#define YT8824_CSR_TX_PRE_SEL			GENMASK(4, 0)
+#define YT8824_CSR_AFE_CTRL1_REG		0x04b0
+#define YT8824_CSR_AFE_CTLE_BW			GENMASK(6, 4)
+#define YT8824_CSR_AFE_CTRL2_REG		0x04b1
+#define YT8824_CSR_AFE_PGA_BW			GENMASK(14, 12)
+#define YT8824_CSR_AFE_BUF_BW			GENMASK(6, 4)
+#define YT8824_CSR_DCC_CAL_CTRL_REG		0x0003
+#define YT8824_CUR_TX_SEQ			GENMASK(15, 12)
+#define YT8824_CUR_RX_SEQ			GENMASK(11, 8)
+#define YT8824_CALIB_SEQ_FINISH			BIT(1)
+#define YT8824_CALIB_SEQ_SW_RST			BIT(0)
+#define YT8824_RESTART_CAL_CTRL_REG		0x2000
+#define YT8824_DAC_TRIM_MAIN_CFG_REG		0x429
+#define YT8824_PLL_VCO_CFG_REG			0x441
+#define YT8824_TX_MUX_CH3_CH2_REG		0x42b
+#define YT8824_C1_COARSE_CFG_REG		0x4b4
+#define YT8824_FFE_CTRL_REG			0x4b5
+
 struct yt8521_priv {
 	/* combo_advertising is used for case of YT8521 in combo mode,
 	 * this means that yt8521 may work in utp or fiber mode which depends
@@ -399,6 +638,12 @@ struct yt8521_priv {
 	u8 reg_page;
 };
 
+struct yt8824_shared_priv {
+	phy_interface_t package_mode;
+	/* shared_lock used to UTPs operation isolation during swap reg space */
+	struct mutex shared_lock;
+};
+
 /**
  * ytphy_read_ext() - read a PHY's extended register
  * @phydev: a pointer to a &struct phy_device
@@ -437,6 +682,70 @@ static int ytphy_read_ext_with_lock(struct phy_device *phydev, u16 regnum)
 	return ret;
 }
 
+/**
+ * ytphy_read_top_ext() - read a PHY's top extended register for YT8824
+ * @phydev: a pointer to a &struct phy_device
+ * @regnum: register number to read
+ *
+ * Returns: the value of regnum reg or negative error code
+ */
+static int ytphy_read_top_ext(struct phy_device *phydev, u16 regnum)
+{
+	int ret;
+
+	lockdep_assert_held(&phydev->mdio.bus->mdio_lock);
+	ret = __phy_package_write(phydev, 0, YTPHY_PAGE_SELECT, regnum);
+	if (ret < 0)
+		return ret;
+
+	return __phy_package_read(phydev, 0, YTPHY_PAGE_DATA);
+}
+
+/**
+ * ytphy_write_top_ext() - write a PHY's top extended register for YT8824
+ * @phydev: a pointer to a &struct phy_device
+ * @regnum: register number to write
+ * @val: register val to write
+ *
+ * Returns: 0 or negative error code
+ */
+static int ytphy_write_top_ext(struct phy_device *phydev, u16 regnum,
+			       u16 val)
+{
+	int ret;
+
+	lockdep_assert_held(&phydev->mdio.bus->mdio_lock);
+	ret = __phy_package_write(phydev, 0, YTPHY_PAGE_SELECT, regnum);
+	if (ret < 0)
+		return ret;
+
+	return __phy_package_write(phydev, 0, YTPHY_PAGE_DATA, val);
+}
+
+/**
+ * phy8824_page_write_with_lock() - write page for YT8824
+ * @phydev: a pointer to a &struct phy_device
+ * @page: reg page(YT8824_RSSR_SERDES_SPACE/YT8824_RSSR_UTP_SPACE).
+ *
+ * Returns: 0 or negative error code
+ */
+static int phy8824_page_write_with_lock(struct phy_device *phydev, int page)
+{
+	int ret;
+
+	phy_lock_mdio_bus(phydev);
+	ret = ytphy_read_top_ext(phydev, YT8521_REG_SPACE_SELECT_REG);
+	if (ret < 0)
+		goto err;
+	ret &= ~YT8824_RSSR_SPACE_MASK;
+	ret |= (page & YT8824_RSSR_SPACE_MASK);
+	ret = ytphy_write_top_ext(phydev, YT8521_REG_SPACE_SELECT_REG, ret);
+
+err:
+	phy_unlock_mdio_bus(phydev);
+	return ret;
+}
+
 /**
  * ytphy_write_ext() - write a PHY's extended register
  * @phydev: a pointer to a &struct phy_device
@@ -633,6 +942,1120 @@ static int ytphy_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
 	return phy_restore_page(phydev, old_page, ret);
 }
 
+/**
+ * yt8824_read_page() - read PHY8824 reg page
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: current reg space of yt8824 (YT8824_RSSR_SERDES_SPACE/
+ * YT8824_RSSR_UTP_SPACE) or negative errno code
+ */
+static int yt8824_read_page(struct phy_device *phydev)
+{
+	int old_page;
+
+	old_page = ytphy_read_top_ext(phydev, YT8521_REG_SPACE_SELECT_REG);
+	if (old_page < 0)
+		return old_page;
+
+	return old_page & YT8824_RSSR_SPACE_MASK;
+};
+
+/**
+ * yt8824_write_page() - write reg page
+ * @phydev: a pointer to a &struct phy_device
+ * @page: Reg page(YT8824_RSSR_SERDES_SPACE/YT8824_RSSR_UTP_SPACE) to write.
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_write_page(struct phy_device *phydev, int page)
+{
+	int old_page;
+	u16 data;
+
+	old_page = ytphy_read_top_ext(phydev, YT8521_REG_SPACE_SELECT_REG);
+	if (old_page < 0)
+		return old_page;
+	data = old_page & (~YT8824_RSSR_SPACE_MASK);
+	data |= page;
+
+	return ytphy_write_top_ext(phydev, YT8521_REG_SPACE_SELECT_REG, data);
+};
+
+/**
+ * yt8824_utp_set_template_test_mode() - config YT8824 UTP test mode.
+ * @phydev: a pointer to a &struct phy_device
+ * @test_mode: template test mode from normal, testmode1 to testmode7
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_utp_set_template_test_mode(struct phy_device *phydev,
+					     u16 test_mode)
+{
+	int ret;
+
+	ret = phy8824_page_write_with_lock(phydev, YT8824_RSSR_UTP_SPACE);
+	if (ret < 0)
+		return ret;
+
+	return genphy_c45_template_testmode(phydev, test_mode);
+}
+
+/**
+ * yt8824_sds_isolate_paged() - enable YT8824 serdes isolate.
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_sds_isolate_paged(struct phy_device *phydev)
+{
+	int old_page = YT8824_RSSR_UTP_SPACE;
+	int ret = 0;
+
+	old_page = phy_select_page(phydev, YT8824_RSSR_SERDES_SPACE);
+	if (old_page < 0)
+		goto err_restore_page;
+
+	/* enable sds isolate */
+	ret = __phy_modify(phydev, MII_BMCR, BMCR_ISOLATE, BMCR_ISOLATE);
+
+err_restore_page:
+	return phy_restore_page(phydev, old_page, ret);
+}
+
+/**
+ * yt8824_utp_softreset_paged() - config YT8824 UTP softreset.
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_utp_softreset_paged(struct phy_device *phydev)
+{
+	int ret = 0;
+	int val;
+
+	ret = phy8824_page_write_with_lock(phydev, YT8824_RSSR_UTP_SPACE);
+	if (ret < 0)
+		return ret;
+	ret = phy_modify(phydev, MII_BMCR, BMCR_RESET, BMCR_RESET);
+	if (ret < 0)
+		return ret;
+	/* wait until softreset done. */
+	return phy_read_poll_timeout(phydev, MII_BMCR, val, !(val & BMCR_RESET),
+				     50000, 600000, true);
+}
+
+/**
+ * yt8824_sds_isolate_and_softreset_paged() - disable YT8824 serdes isolate
+ * and sds softreset.
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_sds_isolate_and_softreset_paged(struct phy_device *phydev)
+{
+	int old_page = YT8824_RSSR_UTP_SPACE;
+	int val = 0;
+	int ret = -1;
+
+	old_page = phy_select_page(phydev, YT8824_RSSR_SERDES_SPACE);
+	if (old_page < 0)
+		goto err_restore_page;
+
+	/* sds softreset and disable isolate */
+	ret = __phy_modify(phydev, MII_BMCR, BMCR_RESET | BMCR_ISOLATE,
+			   BMCR_RESET & ~BMCR_ISOLATE);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* poll while still holding the lock */
+	ret = read_poll_timeout(__phy_read, val,
+				(val < 0) || !(val & BMCR_RESET), 50000, 600000,
+				true, phydev, MII_BMCR);
+	if (val < 0)
+		ret = val;
+
+err_restore_page:
+	return phy_restore_page(phydev, old_page, ret);
+}
+
+/**
+ * yt8824_restore_working_status() - called to do store working status
+ * @phydev: a pointer to a &struct phy_device
+ * @ret: operation's return code
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_restore_working_status(struct phy_device *phydev, int ret)
+{
+	struct yt8824_shared_priv *priv = phy_package_get_priv(phydev);
+	int r;
+
+	r = yt8824_utp_set_template_test_mode(phydev,
+					      MDIO_PMA_10GBT_TESTMODE_NORMAL);
+	if (ret >= 0 && r < 0)
+		ret = r;
+	if (priv->package_mode != PHY_INTERFACE_MODE_INTERNAL) {
+		r = yt8824_sds_isolate_and_softreset_paged(phydev);
+		if (ret >= 0 && r < 0)
+			ret = r;
+	}
+
+	return ret;
+}
+
+/**
+ * yt8824_soft_reset() - called to do PHY software reset
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_soft_reset(struct phy_device *phydev)
+{
+	struct yt8824_shared_priv *priv = phy_package_get_priv(phydev);
+	int ret;
+
+	mutex_lock(&priv->shared_lock);
+	if (priv->package_mode == PHY_INTERFACE_MODE_INTERNAL) {
+		ret = yt8824_utp_set_template_test_mode(phydev,
+							MDIO_PMA_10GBT_TESTMODE_1);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_utp_softreset_paged(phydev);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_utp_set_template_test_mode(phydev,
+							MDIO_PMA_10GBT_TESTMODE_NORMAL);
+		if (ret < 0)
+			goto retry;
+	} else {
+		ret = yt8824_utp_set_template_test_mode(phydev,
+							MDIO_PMA_10GBT_TESTMODE_1);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_sds_isolate_paged(phydev);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_utp_softreset_paged(phydev);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_utp_set_template_test_mode(phydev,
+							MDIO_PMA_10GBT_TESTMODE_NORMAL);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_sds_isolate_and_softreset_paged(phydev);
+		if (ret < 0)
+			goto retry;
+	}
+	mutex_unlock(&priv->shared_lock);
+	return ret;
+retry:
+	ret = yt8824_restore_working_status(phydev, ret);
+	mutex_unlock(&priv->shared_lock);
+
+	return ret;
+}
+
+/**
+ * yt8824_config_utp_init_paged() - config external phy8824 utp init
+ * @phydev: target phy_device struct
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_config_utp_init_paged(struct phy_device *phydev)
+{
+	int ctrl = 0;
+	int mask = 0;
+	int ret = 0;
+	int val = 0;
+	int r;
+
+	ret = phy8824_page_write_with_lock(phydev, YT8824_RSSR_UTP_SPACE);
+	if (ret < 0)
+		return ret;
+
+	ret = phy_modify(phydev, MII_BMCR, BMCR_PDOWN, BMCR_PDOWN);
+	if (ret < 0)
+		goto err_restore;
+
+	/* write protecting */
+	mask = YT8824_WRITE_PROTECT;
+	ctrl = YT8824_WRITE_PROTECT;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_WRITE_PROTECT_CAL_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* PLL calibration */
+	ctrl = FIELD_PREP(YT8824_PLL_VCO_SEL_CURRENT, 0xb);
+	mask = YT8824_PLL_VCO_SEL_CURRENT;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_PLL_VCO_ATEMP_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_PLL_VCO_ICOSTANT, 0xf);
+	mask = YT8824_PLL_VCO_ICOSTANT;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_PLL_PFD_CTRL_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_PLL_VCO_IPTAT, 0xf);
+	mask = YT8824_PLL_VCO_IPTAT;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_PLL_VCO_ATEMP_1_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_PLL_DAC_RST, 0);
+	mask = YT8824_PLL_DAC_RST;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_PLL_DAC_0_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_PLL_DAC_RST, 1);
+	mask = YT8824_PLL_DAC_RST;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_PLL_DAC_0_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* enable nibble */
+	ctrl = FIELD_PREP(YT8824_U0_U1_ENABEL_ODD_PREAMBLE, 3);
+	mask = YT8824_U0_U1_ENABEL_ODD_PREAMBLE;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_IF_CTRL_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* idle err detect enable */
+	ctrl = YT8824_IDLE_DETECT;
+	mask = YT8824_IDLE_DETECT;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_IDLE_CTRL_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* optimized 2.5G long cable performance */
+	ctrl = FIELD_PREP(YT8824_TRACE_LNG_GAIN_THR_2500, 0x50);
+	ctrl |= FIELD_PREP(YT8824_TRACE_MED_GAIN_THR_2500, 0x38);
+	mask = YT8824_TRACE_LNG_GAIN_THR_2500 |
+	       YT8824_TRACE_MED_GAIN_THR_2500;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_TRACE_GAIN_THR_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_PRM_LARGE_SLAVE_2500, 0x68);
+	mask = YT8824_PRM_LARGE_SLAVE_2500;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_PRM_LONG_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_PRM_SMALL_LNG_2500, 0xa0);
+	mask = YT8824_PRM_SMALL_LNG_2500;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_PRM_SMALL_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* optimized fast retrain */
+	ctrl = FIELD_PREP(YT8824_LINK_FAIL_SIG, 0x21);
+	mask = YT8824_LINK_FAIL_SIG;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_FAST_RETARIN_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_EN_GATE_GMII_CLK, 0);
+	mask = YT8824_EN_GATE_GMII_CLK;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_EN_GATE_CTRL_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* 2.5G template tone */
+	ctrl = FIELD_PREP(YT8824_POWER_SAVIE, 0x83);
+	mask = YT8824_POWER_SAVIE;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_CFG_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_DAC_BIAS_CFG_2500, 0xa);
+	mask = YT8824_DAC_BIAS_CFG_2500;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_BIAS_CFG_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* optimized 2.5G template */
+	ctrl = FIELD_PREP(YT8824_DAC_IMID_CH2, 0x39);
+	ctrl |= FIELD_PREP(YT8824_DAC_IMID_CH3, 0x39);
+	mask = YT8824_DAC_IMID_CH2 | YT8824_DAC_IMID_CH3;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_IMID_CFG_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_DAC_IMID_CH0, 0x39);
+	ctrl |= FIELD_PREP(YT8824_DAC_IMID_CH1, 0x39);
+	mask = YT8824_DAC_IMID_CH0 | YT8824_DAC_IMID_CH1;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_IMID_CFG1_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_DAC_IMSB_CH2, 0x39);
+	ctrl |= FIELD_PREP(YT8824_DAC_IMSB_CH3, 0x39);
+	mask = YT8824_DAC_IMSB_CH2 | YT8824_DAC_IMSB_CH3;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_IMSB_CFG_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_DAC_IMSB_CH0, 0x39);
+	ctrl |= FIELD_PREP(YT8824_DAC_IMSB_CH1, 0x39);
+	mask = YT8824_DAC_IMSB_CH0 | YT8824_DAC_IMSB_CH1;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_IMSB_CFG1_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* optimized 1000M cable length threshold */
+	ctrl = FIELD_PREP(YT8824_TRACE_LNG_GAIN_THR, 0x2b);
+	mask = YT8824_TRACE_LNG_GAIN_THR;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_TRACE_LNG_GAIN_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_TRACE_MED_GAIN_THR, 0x1d);
+	mask = YT8824_TRACE_MED_GAIN_THR;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_TRACE_MED_GAIN_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* 100M template amplitude */
+	ctrl = FIELD_PREP(YT8824_DAC_IMID_CH3_ORG, 0x45);
+	ctrl |= FIELD_PREP(YT8824_DAC_IMID_CH2_ORG, 0x45);
+	mask = YT8824_DAC_IMID_CH3_ORG | YT8824_DAC_IMID_CH3_ORG;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_IMID_CH2_CH3_CFG_ORG_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_DAC_IMID_CH1_ORG, 0x45);
+	ctrl |= FIELD_PREP(YT8824_DAC_IMID_CH0_ORG, 0x45);
+	mask = YT8824_DAC_IMID_CH1_ORG | YT8824_DAC_IMID_CH0_ORG;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_IMID_CH0_CH1_CFG_ORG_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_DAC_IMSB_CH3_ORG, 0x45);
+	ctrl |= FIELD_PREP(YT8824_DAC_IMSB_CH2_ORG, 0x45);
+	mask = YT8824_DAC_IMSB_CH2_ORG | YT8824_DAC_IMSB_CH3_ORG;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_IMSB_CH2_CH3_CFG_ORG_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_DAC_IMSB_CH1_ORG, 0x45);
+	ctrl |= FIELD_PREP(YT8824_DAC_IMSB_CH0_ORG, 0x45);
+	mask = YT8824_DAC_IMSB_CH1_ORG | YT8824_DAC_IMSB_CH0_ORG;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_IMSB_CH0_CH1_CFG_ORG_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* optimized 100M cable length threshold */
+	ctrl = FIELD_PREP(YT8824_TRACE_LNG_GAIN_THR_100, 0x2A);
+	ctrl |= FIELD_PREP(YT8824_TRACE_MED_GAIN_THR_100, 0x1D);
+	mask = YT8824_TRACE_LNG_GAIN_THR_100 | YT8824_TRACE_MED_GAIN_THR_100;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_TRACE_LNG_GAIN_100_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_CABLE_VSHT_TH_S, 0x36);
+	mask = YT8824_CABLE_VSHT_TH_S;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_CABLE_VSHT_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* 10M template amplitude */
+	ctrl = FIELD_PREP(YT8824_DAC_LPFIL_ADJ_CH3_10, 0x18);
+	ctrl |= FIELD_PREP(YT8824_DAC_LPFIL_ADJ_CH2_10, 0x18);
+	mask = YT8824_DAC_LPFIL_ADJ_CH3_10 | YT8824_DAC_LPFIL_ADJ_CH2_10;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_LPFIL_CH2_CH3_ADJ_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_DAC_LPFIL_ADJ_CH1_10, 0x18);
+	ctrl |= FIELD_PREP(YT8824_DAC_LPFIL_ADJ_CH0_10, 0x18);
+	mask = YT8824_DAC_LPFIL_ADJ_CH1_10 | YT8824_DAC_LPFIL_ADJ_CH0_10;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_LPFIL_CH0_CH1_ADJ_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* optimized 10M cable length threshold */
+	ctrl = FIELD_PREP(YT8824_DAC_IMID_CFG_CH3_10_ORG, 0x6C);
+	ctrl |= FIELD_PREP(YT8824_DAC_IMID_CFG_CH2_10_ORG, 0x6C);
+	mask = YT8824_DAC_IMID_CFG_CH3_10_ORG | YT8824_DAC_IMID_CFG_CH2_10_ORG;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_IMID_CFG_CH2_CH3_10_ORG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_DAC_IMID_CFG_CH1_10_ORG, 0x6C);
+	ctrl |= FIELD_PREP(YT8824_DAC_IMID_CFG_CH0_10_ORG, 0x6C);
+	mask = YT8824_DAC_IMID_CFG_CH1_10_ORG | YT8824_DAC_IMID_CFG_CH0_10_ORG;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_IMID_CFG_CH0_CH1_10_ORG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* optimized DAC IMSB config for CH2/CH3 */
+	ctrl = FIELD_PREP(YT8824_DAC_IMSB_CFG_CH3_10_ORG, 0x6C);
+	ctrl |= FIELD_PREP(YT8824_DAC_IMSB_CFG_CH2_10_ORG, 0x6C);
+	mask = YT8824_DAC_IMSB_CFG_CH3_10_ORG | YT8824_DAC_IMSB_CFG_CH2_10_ORG;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_IMSB_CFG_CH2_CH3_10_ORG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* optimized DAC IMSB config for CH0/CH1 */
+	ctrl = FIELD_PREP(YT8824_DAC_IMSB_CFG_CH1_10_ORG, 0x6C);
+	ctrl |= FIELD_PREP(YT8824_DAC_IMSB_CFG_CH0_10_ORG, 0x6C);
+	mask = YT8824_DAC_IMSB_CFG_CH1_10_ORG | YT8824_DAC_IMSB_CFG_CH0_10_ORG;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_IMSB_CFG_CH0_CH1_10_ORG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* optimize utp 1000M performance */
+	ctrl = FIELD_PREP(YT8824_WAIT_TO_1000, 0xFF);
+	mask = YT8824_WAIT_TO_1000;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_FFE_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_EN_GATE_PHYDBG, 0);
+	ctrl |= FIELD_PREP(YT8824_EN_GATE_BT1000, 0);
+	mask = YT8824_EN_GATE_PHYDBG | YT8824_EN_GATE_BT1000;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_EN_GATE_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_CNT_ERR_AUTO, 1);
+	mask = YT8824_CNT_ERR_AUTO;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_CNT_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_PRM_LARGE_MASTER_1000, 0x94);
+	mask = YT8824_PRM_LARGE_MASTER_1000;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_PRM_LARGE_MASTER_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_PRM_LARGE_SLAVE_1000, 0x094);
+	mask = YT8824_PRM_LARGE_SLAVE_1000;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_PRM_LARGE_SALVE_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_SIG_GONE_THR_1000, 0x08);
+	mask = YT8824_SIG_GONE_THR_1000;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_SIG_GONE_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_EC_NX_TRAIN_COARSE_TO_1000, 0x40);
+	mask = YT8824_EC_NX_TRAIN_COARSE_TO_1000;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_EN_NX_TRAIN_COARSE_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_VGA_IN_LPF2_CAP_OTHER, 0x2);
+	mask = YT8824_VGA_IN_LPF2_CAP_OTHER;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_VGA_IN_LPF2_CAP_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_VGA_IN_LPF1_CAP_OTHER, 0x2);
+	mask = YT8824_VGA_IN_LPF1_CAP_OTHER;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_VGA_IN_LPF1_CAP_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* optimized EMC CS */
+	ctrl = FIELD_PREP(YT8824_LDPC_FAIL_TH, 0xFF);
+	ctrl |= FIELD_PREP(YT8824_LFER_FAIL_TH, 0xFF);
+	mask = YT8824_LDPC_FAIL_TH | YT8824_LFER_FAIL_TH;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_LDPC_LFER_FAIL_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_FR_REQ_TH, 0x06);
+	mask = YT8824_FR_REQ_TH;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_FR_REQ_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_PRM_SMALL_SHT_2500, 0x0FF);
+	mask = YT8824_PRM_SMALL_SHT_2500;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_GN_MU_CPARSE_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	/* optimized EMC RE */
+	ctrl = FIELD_PREP(YT8824_DAC_AMP_ADJ_CH3_PBO0_2500, 0xF);
+	ctrl |= FIELD_PREP(YT8824_DAC_AMP_ADJ_CH2_PBO0_2500, 0xF);
+	ctrl |= FIELD_PREP(YT8824_DAC_AMP_ADJ_CH1_PBO0_2500, 0xF);
+	ctrl |= FIELD_PREP(YT8824_DAC_AMP_ADJ_CH0_PBO0_2500, 0xF);
+	mask = YT8824_DAC_AMP_ADJ_CH3_PBO0_2500 |
+	       YT8824_DAC_AMP_ADJ_CH2_PBO0_2500 |
+	       YT8824_DAC_AMP_ADJ_CH1_PBO0_2500 |
+	       YT8824_DAC_AMP_ADJ_CH0_PBO0_2500;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_DAC_AMP_ADJ_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_P0_DAC_BIAS_CAS_CFG_CH3, 0x1F);
+	ctrl |= FIELD_PREP(YT8824_P0_DAC_BIAS_CAS_CFG_CH2, 0x1F);
+	mask = YT8824_P0_DAC_BIAS_CAS_CFG_CH3 | YT8824_P0_DAC_BIAS_CAS_CFG_CH2;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_P0_DAC_BIAS_CAS_CH3_CH2_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_P0_DAC_BIAS_CAS_CFG_CH1, 0x1F);
+	ctrl |= FIELD_PREP(YT8824_P0_DAC_BIAS_CAS_CFG_CH0, 0x1F);
+	mask = YT8824_P0_DAC_BIAS_CAS_CFG_CH1 | YT8824_P0_DAC_BIAS_CAS_CFG_CH0;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_P0_DAC_BIAS_CAS_CH1_CH0_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_P1_DAC_BIAS_CAS_CFG_CH3, 0x1F);
+	ctrl |= FIELD_PREP(YT8824_P1_DAC_BIAS_CAS_CFG_CH2, 0x1F);
+	mask = YT8824_P1_DAC_BIAS_CAS_CFG_CH3 | YT8824_P1_DAC_BIAS_CAS_CFG_CH2;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_P1_DAC_BIAS_CAS_CH3_CH2_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_P1_DAC_BIAS_CAS_CFG_CH1, 0x1F);
+	ctrl |= FIELD_PREP(YT8824_P1_DAC_BIAS_CAS_CFG_CH0, 0x1F);
+	mask = YT8824_P1_DAC_BIAS_CAS_CFG_CH1 | YT8824_P1_DAC_BIAS_CAS_CFG_CH0;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_P1_DAC_BIAS_CAS_CH1_CH0_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_CSR_EFUSE_BYPASS_ANALOG_SECOND_RESET, 1);
+	ctrl |= FIELD_PREP(YT8824_CSR_EFUSE_BYPASS_PLL_TXRX_RESET, 1);
+	mask  = YT8824_CSR_EFUSE_BYPASS_ANALOG_SECOND_RESET |
+		YT8824_CSR_EFUSE_BYPASS_PLL_TXRX_RESET;
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8824_P01_PLL_EN_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH0, 0);
+	ctrl |= FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH1, 0);
+	ctrl |= FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH2, 0);
+	ctrl |= FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH3, 0);
+	mask  = YT8824_U0_CSR_RESETB_TX_CH0 | YT8824_U0_CSR_RESETB_TX_CH1 |
+		YT8824_U0_CSR_RESETB_TX_CH2 | YT8824_U0_CSR_RESETB_TX_CH3;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_U0_CRS_RESET_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_U1_CSR_RESETB_TX_CH0, 0);
+	ctrl |= FIELD_PREP(YT8824_U1_CSR_RESETB_TX_CH1, 0);
+	ctrl |= FIELD_PREP(YT8824_U1_CSR_RESETB_TX_CH2, 0);
+	ctrl |= FIELD_PREP(YT8824_U1_CSR_RESETB_TX_CH3, 0);
+	mask = YT8824_U1_CSR_RESETB_TX_CH0 | YT8824_U1_CSR_RESETB_TX_CH1 |
+	       YT8824_U1_CSR_RESETB_TX_CH2 | YT8824_U1_CSR_RESETB_TX_CH3;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_U1_CRS_RESET_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	ctrl = FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH0, 1);
+	ctrl |= FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH1, 1);
+	ctrl |= FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH2, 1);
+	ctrl |= FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH3, 1);
+	mask = YT8824_U0_CSR_RESETB_TX_CH0 | YT8824_U0_CSR_RESETB_TX_CH1 |
+	       YT8824_U0_CSR_RESETB_TX_CH2 | YT8824_U0_CSR_RESETB_TX_CH3;
+
+	ctrl = FIELD_PREP(YT8824_U1_CSR_RESETB_TX_CH0, 1);
+	ctrl |= FIELD_PREP(YT8824_U1_CSR_RESETB_TX_CH1, 1);
+	ctrl |= FIELD_PREP(YT8824_U1_CSR_RESETB_TX_CH2, 1);
+	ctrl |= FIELD_PREP(YT8824_U1_CSR_RESETB_TX_CH3, 1);
+	mask = YT8824_U1_CSR_RESETB_TX_CH0 | YT8824_U1_CSR_RESETB_TX_CH1 |
+	       YT8824_U1_CSR_RESETB_TX_CH2 | YT8824_U1_CSR_RESETB_TX_CH3;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_U1_CRS_RESET_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+	/* reset */
+	ret = genphy_c45_template_testmode(phydev, MDIO_PMA_10GBT_TESTMODE_1);
+	if (ret < 0)
+		goto err_restore_normal;
+
+	ret = phy_modify(phydev, MII_BMCR, BMCR_RESET | BMCR_ANENABLE,
+			 BMCR_RESET | BMCR_ANENABLE);
+	if (ret < 0)
+		goto err_restore_normal;
+	ret = phy_read_poll_timeout(phydev, MII_BMCR, val, !(val & BMCR_RESET),
+				    50000, 600000, true);
+	if (ret < 0)
+		goto err_restore_normal;
+
+	ret = genphy_c45_template_testmode(phydev,
+					   MDIO_PMA_10GBT_TESTMODE_NORMAL);
+	if (ret < 0)
+		goto err_restore_normal;
+	return 0;
+
+err_restore:
+	r = phy_modify(phydev, MII_BMCR, BMCR_PDOWN, 0);
+	if (ret >= 0 && r < 0)
+		ret = r;
+	return ret;
+
+err_restore_normal:
+	r = genphy_c45_template_testmode(phydev,
+					 MDIO_PMA_10GBT_TESTMODE_NORMAL);
+	if (ret >= 0 && r < 0)
+		ret = r;
+	r = phy_modify(phydev, MII_BMCR, BMCR_PDOWN, 0);
+	if (ret >= 0 && r < 0)
+		ret = r;
+	return ret;
+}
+
+/**
+ * yt8824_extern_config_sds_init_paged() - config external phy8824 sds init
+ * @phydev: target phy_device struct
+ *
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_extern_config_sds_init_paged(struct phy_device *phydev)
+{
+	int old_page = YT8824_RSSR_UTP_SPACE;
+	int val_1, val_2, val_3, tmp;
+	int ctrl = 0;
+	int mask = 0;
+	int ret = -1;
+	int val;
+
+	old_page = phy_select_page(phydev, YT8824_RSSR_SERDES_SPACE);
+	if (old_page < 0)
+		goto err_restore_page;
+
+	/* read efuse */
+	ret = ytphy_read_top_ext(phydev, YT8824_SERDES_PLL_CTRL1_REG);
+	if (ret < 0)
+		goto err_restore_page;
+	else
+		val_1 = ret;
+
+	ret = ytphy_read_top_ext(phydev, YT8824_SERDES_PLL_CTRL2_REG);
+	if (ret < 0)
+		goto err_restore_page;
+	else
+		val_2 = ret;
+
+	ret = ytphy_read_top_ext(phydev, YT8824_SERDES_PLL_CTRL3_REG);
+	if (ret < 0)
+		goto err_restore_page;
+	else
+		val_3 = ret;
+
+	/* Serdes optimization */
+	ctrl = FIELD_PREP(YT8824_SERDES_REG_WR_PRT, 1);
+	mask = YT8824_SERDES_REG_WR_PRT;
+	ret = ytphy_modify_ext(phydev, YT8824_WRITE_PROTECT_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_CSR_PCS_GLB_RST_BYPASS, 1);
+	ctrl |= FIELD_PREP(YT8824_CSR_SG_MODE_RST_BYPASS, 1);
+	ctrl |= FIELD_PREP(YT8824_CSR_US_GLB_RESET_N, 0);
+	ctrl |= FIELD_PREP(YT8824_CSR_PCS_BEF_TRAINING_DONE_RST_BYPASS, 1);
+	mask = YT8824_CSR_PCS_GLB_RST_BYPASS | YT8824_CSR_SG_MODE_RST_BYPASS |
+	       YT8824_CSR_US_GLB_RESET_N |
+	       YT8824_CSR_PCS_BEF_TRAINING_DONE_RST_BYPASS;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_CSR_WATCH_DOG_ALL_BYPASS, 1);
+	mask = YT8824_CSR_WATCH_DOG_ALL_BYPASS;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_PMA_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* analog CDR */
+	ctrl = FIELD_PREP(YT8824_CDR_MODE, 1);
+	mask = YT8824_CDR_MODE;
+	ret = ytphy_modify_ext(phydev, YT8824_CDR_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* optimized VCO */
+	ctrl = FIELD_PREP(YT8824_CSR_PLL_VCO_SEL_VCO2_CURRENT, 0x9);
+	ctrl |= FIELD_PREP(YT8824_CSR_PLL_VCO_ATEMP_R, 0x4);
+	mask = YT8824_CSR_PLL_VCO_SEL_VCO2_CURRENT | YT8824_CSR_PLL_VCO_ATEMP_R;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_PLL_VCO_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_CSR_PLL_VCO_SEL_VCO2_RM_R, 0x0);
+	mask = YT8824_CSR_PLL_VCO_SEL_VCO2_RM_R;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_PLL_DIVLDO_CTRL_REG,
+			       mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* optimized PLL lock */
+	ret = ytphy_read_ext(phydev, YT8824_DAC_TRIM_MAIN_CFG_REG);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ret &= ~(BIT(13) | BIT(12));
+	tmp = (val_1 & (BIT(7) | BIT(6))) >> 6;
+	ret |= (tmp << 12);
+	ret = ytphy_write_ext(phydev, YT8824_DAC_TRIM_MAIN_CFG_REG, ret);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ret = ytphy_read_ext(phydev, YT8824_PLL_VCO_CFG_REG);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ret &= ~(BIT(1) | BIT(0));
+	tmp = (val_1 & (BIT(5) | BIT(4))) >> 4;
+	ret |= tmp;
+	ret = ytphy_write_ext(phydev, YT8824_PLL_VCO_CFG_REG, ret);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ret = ytphy_read_ext(phydev, YT8824_TX_MUX_CH3_CH2_REG);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ret &= ~(BIT(13) | BIT(12));
+	tmp = (val_3 & (BIT(1) | BIT(0)));
+	ret |= (tmp << 12);
+	ret = ytphy_write_ext(phydev, YT8824_TX_MUX_CH3_CH2_REG, ret);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_CSR_PLL_FBDIV_SYNC_SELB, 1);
+	mask = YT8824_CSR_PLL_FBDIV_SYNC_SELB;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_PLL_FBDIV_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_CSR_PLL_SPARE, 0xf070);
+	mask = YT8824_CSR_PLL_SPARE;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_PLL_SPARE_CTRL_REG,
+			       mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* cable length threshold */
+	ctrl = FIELD_PREP(YT8824_VDAC_AMP_TH_LNG, 0x7F);
+	mask = YT8824_VDAC_AMP_TH_LNG;
+	ret = ytphy_modify_ext(phydev, YT8824_CDR_MANUAL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_VDAC_AMP_TH_MED, 0x7F);
+	ctrl |= FIELD_PREP(YT8824_VDAC_AMP_TH_SHT, 0x7F);
+	mask = YT8824_VDAC_AMP_TH_MED | YT8824_VDAC_AMP_TH_SHT;
+	ret = ytphy_modify_ext(phydev, YT8824_TRAINING_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* Serdes training threshold */
+	ctrl = FIELD_PREP(YT8824_EOD_DURATION_SEL, 0xF);
+	ctrl |= FIELD_PREP(YT8824_EOD_VTH, 0x14);
+	mask = YT8824_EOD_DURATION_SEL | YT8824_EOD_VTH;
+	ret = ytphy_modify_ext(phydev, YT8824_EOM_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_CDR_OFFSET_ERR_WTH, 0x0A);
+	ctrl |= FIELD_PREP(YT8824_PGA_GAIN_MAX, 0x4);
+	ctrl |= FIELD_PREP(YT8824_PGA_GAIN_INIT, 0x4);
+	mask = YT8824_CDR_OFFSET_ERR_WTH | YT8824_PGA_GAIN_MAX |
+	       YT8824_PGA_GAIN_INIT;
+	ret = ytphy_modify_ext(phydev, YT8824_PGA_GAIN_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* digital eye diagram of SerDes */
+	ctrl = FIELD_PREP(YT8824_EOD_ERR_CNT_TH, 0x0000);
+	mask = YT8824_EOD_ERR_CNT_TH;
+	ret = ytphy_modify_ext(phydev, YT8824_EOD_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* Serdes LDO */
+	ret = ytphy_read_ext(phydev, YT8824_FFE_CTRL_REG);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ret &= ~(BIT(6) | BIT(5) | BIT(4));
+	tmp = (val_2 & (BIT(4) | BIT(3) | BIT(2))) >> 2;
+	ret |= (tmp << 4);
+	ret = ytphy_write_ext(phydev, YT8824_FFE_CTRL_REG, ret);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ret = ytphy_read_ext(phydev, YT8824_C1_COARSE_CFG_REG);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ret &= ~(BIT(10) | BIT(9) | BIT(8));
+	tmp = (val_2 & (BIT(7) | BIT(6) | BIT(5))) >> 5;
+	ret |= (tmp << 8);
+	ret = ytphy_write_ext(phydev, YT8824_C1_COARSE_CFG_REG, ret);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* optimized Serdes RX */
+	ctrl = FIELD_PREP(YT8824_CSR_AFE_VCM_CFG, 0x5);
+	ctrl |= FIELD_PREP(YT8824_CSR_AFE_MANUAL_CTRL_1P25G, 0);
+	ctrl |= FIELD_PREP(YT8824_CSR_AFE_MANUAL_CTRL_3P125G, 0);
+	mask = YT8824_CSR_AFE_VCM_CFG | YT8824_CSR_AFE_MANUAL_CTRL_1P25G |
+	       YT8824_CSR_AFE_MANUAL_CTRL_3P125G;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_AFE_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_DFE_TAP_POLARITY, 1);
+	ctrl |= FIELD_PREP(YT8824_DFE_IDAC_TUNE, 0x3);
+	mask = YT8824_DFE_TAP_POLARITY | YT8824_DFE_IDAC_TUNE;
+	ret = ytphy_modify_ext(phydev, YT8824_DFE_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_CSR_CDR_OFFSET_DATA, 0x7C);
+	mask = YT8824_CSR_CDR_OFFSET_DATA;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_CDR_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_DFE_VDAC_CODE_TH, 0x7F);
+	mask = YT8824_DFE_VDAC_CODE_TH;
+	ret = ytphy_modify_ext(phydev, YT8824_DFE_VDAC_CODE_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_RX_SPARE_REG, 0xff08);
+	mask = YT8824_RX_SPARE_REG;
+	ret = ytphy_modify_ext(phydev, YT8824_RX_SPARE_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* optimized Serdes TX */
+	ctrl = FIELD_PREP(YT8824_DFE_VDAC_TUNE, 0x1);
+	ctrl |= FIELD_PREP(YT8824_DFE_VDAC_SEL, 0x0);
+	mask = YT8824_DFE_VDAC_TUNE | YT8824_DFE_VDAC_SEL;
+	ret = ytphy_modify_ext(phydev, YT8824_DFE_VDAC_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_CSR_TX_PRE_SEL, 0x6);
+	mask = YT8824_CSR_TX_PRE_SEL;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_TX_PRE_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* Serdes manual config */
+	ctrl = FIELD_PREP(YT8824_CSR_AFE_CTLE_BW, 0x0);
+	mask = YT8824_CSR_AFE_CTLE_BW;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_AFE_CTRL1_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_CSR_AFE_PGA_BW, 0x7);
+	ctrl |= FIELD_PREP(YT8824_CSR_AFE_BUF_BW, 0x7);
+	mask = YT8824_CSR_AFE_PGA_BW | YT8824_CSR_AFE_BUF_BW;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_AFE_CTRL2_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_CSR_AFE_MANUAL_CTRL_10P3125G, 0x1);
+	mask = YT8824_CSR_AFE_MANUAL_CTRL_10P3125G;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_AFE_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* restart calibration */
+	ctrl = FIELD_PREP(YT8824_CUR_TX_SEQ, 0x5);
+	ctrl |= FIELD_PREP(YT8824_CUR_RX_SEQ, 0x6);
+	ctrl |= FIELD_PREP(YT8824_CALIB_SEQ_FINISH, 1);
+	ctrl |= FIELD_PREP(YT8824_CALIB_SEQ_SW_RST, 1);
+	mask = YT8824_CUR_TX_SEQ | YT8824_CUR_RX_SEQ |
+	       YT8824_CALIB_SEQ_FINISH | YT8824_CALIB_SEQ_SW_RST;
+	ret = ytphy_modify_ext(phydev, YT8824_CSR_DCC_CAL_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_VDAC_AMP_TH_MED, 0x7F);
+	ctrl |= FIELD_PREP(YT8824_VDAC_AMP_TH_SHT, 0xFF);
+	mask = YT8824_VDAC_AMP_TH_MED | YT8824_VDAC_AMP_TH_SHT;
+	ret = ytphy_modify_ext(phydev, YT8824_TRAINING_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ctrl = FIELD_PREP(YT8824_VDAC_AMP_TH_MED, 0x7F);
+	ctrl |= FIELD_PREP(YT8824_VDAC_AMP_TH_SHT, 0x7F);
+	mask = YT8824_VDAC_AMP_TH_MED | YT8824_VDAC_AMP_TH_SHT;
+	ret = ytphy_modify_ext(phydev, YT8824_TRAINING_CTRL_REG, mask, ctrl);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* restart to calibration */
+	ret = ytphy_write_ext(phydev, YT8824_RESTART_CAL_CTRL_REG, 0x0040);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ret = ytphy_write_ext(phydev, YT8824_RESTART_CAL_CTRL_REG, 0x0);
+	if (ret < 0)
+		goto err_restore_page;
+
+	/* TX preamble padded to 8; RX IPG always > 8 */
+	ret = __phy_read(phydev, MII_RESV1);
+	if (ret < 0)
+		goto err_restore_page;
+	ret &= ~YT8824_SDS_CFG_MIN_PRE_MASK;
+	ret |= YT8824_SDS_TX_PRE_PADDING;
+	ret |= YT8824_SDS_EN_FILL_PRE;
+	ret = __phy_write(phydev, MII_RESV1, ret);
+	if (ret < 0)
+		goto err_restore_page;
+
+	ret = __phy_modify(phydev, MII_BMCR, BMCR_RESET | BMCR_ANENABLE,
+			   BMCR_RESET | BMCR_ANENABLE);
+	if (ret < 0)
+		goto err_restore_page;
+	/* poll while still holding the lock; __phy_read takes no lock */
+	ret = read_poll_timeout(__phy_read, val,
+				(val < 0) || !(val & BMCR_RESET), 50000, 600000,
+				true, phydev, MII_BMCR);
+	if (val < 0)
+		ret = val;
+err_restore_page:
+	return phy_restore_page(phydev, old_page, ret);
+}
+
+/**
+ * yt8824_config_init() - phy initializatioin
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_config_init(struct phy_device *phydev)
+{
+	struct yt8824_shared_priv *priv = phy_package_get_priv(phydev);
+	int ret;
+
+	mutex_lock(&priv->shared_lock);
+	if (priv->package_mode == PHY_INTERFACE_MODE_INTERNAL) {
+		ret = yt8824_config_utp_init_paged(phydev);
+		if (ret < 0)
+			goto err;
+	} else {
+		ret = yt8824_extern_config_sds_init_paged(phydev);
+		if (ret < 0)
+			goto err;
+		ret = yt8824_config_utp_init_paged(phydev);
+		if (ret < 0)
+			goto err;
+	}
+	mutex_unlock(&priv->shared_lock);
+	ret = yt8824_soft_reset(phydev);
+
+	phydev_dbg(phydev, "%s done, phy addr: %d\n", __func__,
+		   phydev->mdio.addr);
+	return ret;
+err:
+	mutex_unlock(&priv->shared_lock);
+	return ret;
+}
+
 static int yt8531_set_wol(struct phy_device *phydev,
 			  struct ethtool_wolinfo *wol)
 {
@@ -3104,6 +4527,449 @@ static int yt8821_resume(struct phy_device *phydev)
 	return yt8821_modify_utp_fiber_bmcr(phydev, BMCR_PDOWN, 0);
 }
 
+/**
+ * yt8824_get_features - read mmd register to get 2.5G capability
+ * @phydev: target phy_device struct
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_get_features(struct phy_device *phydev)
+{
+	struct yt8824_shared_priv *priv = phy_package_get_priv(phydev);
+	int ret;
+
+	mutex_lock(&priv->shared_lock);
+	ret = phy8824_page_write_with_lock(phydev, YT8824_RSSR_UTP_SPACE);
+	if (ret < 0)
+		goto err;
+	ret = yt8821_get_features(phydev);
+
+err:
+	mutex_unlock(&priv->shared_lock);
+	return ret;
+}
+
+/**
+ * yt8824_aneg_done()  - check negotiation state.
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: auto-negotiation complete status or negative errno code
+ */
+static int yt8824_aneg_done(struct phy_device *phydev)
+{
+	struct yt8824_shared_priv *priv = phy_package_get_priv(phydev);
+	int auto_neg;
+	int ret;
+
+	mutex_lock(&priv->shared_lock);
+	ret = phy8824_page_write_with_lock(phydev, YT8824_RSSR_UTP_SPACE);
+	if (ret < 0)
+		goto err;
+
+	ret = phy_read(phydev, MII_BMSR);
+	if (ret < 0)
+		goto err;
+	mutex_unlock(&priv->shared_lock);
+	auto_neg = !!(ret & BMSR_ANEGCOMPLETE);
+
+	phydev_dbg(phydev, "%s, phy addr: %d, auto negotiation done: %d\n",
+		   __func__, phydev->mdio.addr, auto_neg);
+	return auto_neg;
+err:
+	mutex_unlock(&priv->shared_lock);
+	return ret;
+}
+
+/**
+ * yt8824_read_status_paged() -  determines the speed and duplex of one page
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_read_status_paged(struct phy_device *phydev)
+{
+	int ret;
+	int val;
+
+	ret = phy8824_page_write_with_lock(phydev, YT8824_RSSR_UTP_SPACE);
+	if (ret < 0)
+		return ret;
+
+	ret = genphy_read_status(phydev);
+	if (ret < 0)
+		return ret;
+
+	if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
+		ret = genphy_c45_read_lpa(phydev);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (!phydev->link) {
+		phydev->speed = SPEED_UNKNOWN;
+		phydev->duplex = DUPLEX_UNKNOWN;
+		if (phydev->autoneg == AUTONEG_ENABLE)
+			phy_resolve_aneg_pause(phydev);
+		return 0;
+	}
+
+	ret = phy_read(phydev, YTPHY_SPECIFIC_STATUS_REG);
+	if (ret < 0)
+		return ret;
+
+	val = ret;
+
+	yt8821_adjust_status(phydev, val);
+
+	if (phydev->autoneg == AUTONEG_ENABLE)
+		phy_resolve_aneg_pause(phydev);
+
+	return 0;
+}
+
+/**
+ * yt8824_read_status() -  determines the negotiated speed and duplex
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_read_status(struct phy_device *phydev)
+{
+	struct yt8824_shared_priv *priv = phy_package_get_priv(phydev);
+	int ret;
+
+	mutex_lock(&priv->shared_lock);
+	ret = yt8824_read_status_paged(phydev);
+	mutex_unlock(&priv->shared_lock);
+
+	return ret;
+}
+
+/**
+ * yt8824_utp_power_on(): utp power on.
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_utp_power_on(struct phy_device *phydev)
+{
+	int ret = 0;
+
+	ret = phy8824_page_write_with_lock(phydev, YT8824_RSSR_UTP_SPACE);
+	if (ret < 0)
+		return ret;
+
+	return phy_modify(phydev, MII_BMCR, BMCR_PDOWN | BMCR_ISOLATE, 0x0);
+}
+
+/**
+ * yt8824_utp_power_down(): utp power down.
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_utp_power_down(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = phy8824_page_write_with_lock(phydev, YT8824_RSSR_UTP_SPACE);
+	if (ret < 0)
+		return ret;
+
+	return phy_modify(phydev, MII_BMCR, BMCR_PDOWN, BMCR_PDOWN);
+}
+
+/**
+ * yt8824_power_on()  - set utp power on.
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * NOTE: need WA like softreset
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_power_on(struct phy_device *phydev)
+{
+	struct yt8824_shared_priv *priv = phy_package_get_priv(phydev);
+	int ret;
+	int r;
+
+	if (priv->package_mode == PHY_INTERFACE_MODE_INTERNAL) {
+		ret = yt8824_utp_set_template_test_mode(phydev,
+							MDIO_PMA_10GBT_TESTMODE_1);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_utp_power_on(phydev);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_utp_set_template_test_mode(phydev,
+							MDIO_PMA_10GBT_TESTMODE_NORMAL);
+		if (ret < 0)
+			goto retry;
+	} else {
+		ret = yt8824_utp_set_template_test_mode(phydev,
+							MDIO_PMA_10GBT_TESTMODE_1);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_sds_isolate_paged(phydev);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_utp_power_on(phydev);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_utp_set_template_test_mode(phydev,
+							MDIO_PMA_10GBT_TESTMODE_NORMAL);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_sds_isolate_and_softreset_paged(phydev);
+		if (ret < 0)
+			goto retry;
+	}
+	return 0;
+
+retry:
+	/*
+	 * If the PHY up operation succeeds but the subsequent operation
+	 * fails, revert to the default state.
+	 */
+	r = yt8824_utp_power_down(phydev);
+	if (ret >= 0 && r < 0)
+		ret = r;
+	ret = yt8824_restore_working_status(phydev, ret);
+	return ret;
+}
+
+/**
+ * yt8824_resume() - resume the hardware
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_resume(struct phy_device *phydev)
+{
+	struct yt8824_shared_priv *priv = phy_package_get_priv(phydev);
+	int ret;
+
+	mutex_lock(&priv->shared_lock);
+	ret = yt8824_power_on(phydev);
+	mutex_unlock(&priv->shared_lock);
+
+	return ret;
+}
+
+/**
+ * yt8824_power_down()  - set utp power down.
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * NOTE: need WA like softreset
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_power_down(struct phy_device *phydev)
+{
+	struct yt8824_shared_priv *priv = phy_package_get_priv(phydev);
+	int ret;
+	int r;
+
+	if (priv->package_mode == PHY_INTERFACE_MODE_INTERNAL) {
+		ret = yt8824_utp_set_template_test_mode(phydev,
+							MDIO_PMA_10GBT_TESTMODE_1);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_utp_power_down(phydev);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_utp_set_template_test_mode(phydev,
+							MDIO_PMA_10GBT_TESTMODE_NORMAL);
+		if (ret < 0)
+			goto retry;
+	} else {
+		ret = yt8824_utp_set_template_test_mode(phydev,
+							MDIO_PMA_10GBT_TESTMODE_1);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_sds_isolate_paged(phydev);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_utp_power_down(phydev);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_utp_set_template_test_mode(phydev,
+							MDIO_PMA_10GBT_TESTMODE_NORMAL);
+		if (ret < 0)
+			goto retry;
+
+		ret = yt8824_sds_isolate_and_softreset_paged(phydev);
+		if (ret < 0)
+			goto retry;
+	}
+	return 0;
+
+retry:
+	/*
+	 * If the PHY down operation succeeds but the subsequent operation
+	 * fails, revert to the default state.
+	 */
+	r = yt8824_utp_power_on(phydev);
+	if (ret >= 0 && r < 0)
+		ret = r;
+	ret = yt8824_restore_working_status(phydev, ret);
+	return ret;
+}
+
+/**
+ * yt8824_suspend() - suspend the hardware
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_suspend(struct phy_device *phydev)
+{
+	struct yt8824_shared_priv *priv = phy_package_get_priv(phydev);
+	int ret;
+
+	mutex_lock(&priv->shared_lock);
+	ret = yt8824_power_down(phydev);
+	mutex_unlock(&priv->shared_lock);
+
+	return ret;
+}
+
+/**
+ * yt8824_config_aneg() - config negotiation
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_config_aneg(struct phy_device *phydev)
+{
+	struct yt8824_shared_priv *priv = phy_package_get_priv(phydev);
+	int phy_ctrl = 0;
+	int ret;
+
+	mutex_lock(&priv->shared_lock);
+	ret = phy8824_page_write_with_lock(phydev, YT8824_RSSR_UTP_SPACE);
+	if (ret < 0)
+		goto err;
+
+	/*
+	 * Only advertise 2.5G when autoneg is enabled, or when 2.5G is
+	 * explicitly forced.  When a different speed is forced, clear
+	 * ADV2_5G so a 2.5G-capable link partner cannot negotiate 2.5G.
+	 * __genphy_config_aneg() only rewrites the
+	 * clause 22 registers on the forced-speed path, so it will not
+	 * clear this bit.
+	 */
+	if ((phydev->autoneg == AUTONEG_ENABLE ||
+	     phydev->speed == SPEED_2500) &&
+	    linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+			      phydev->advertising))
+		phy_ctrl = MDIO_AN_10GBT_CTRL_ADV2_5G;
+
+	ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL,
+				     MDIO_AN_10GBT_CTRL_ADV2_5G, phy_ctrl);
+	if (ret < 0)
+		goto err;
+
+	ret = __genphy_config_aneg(phydev, ret);
+
+err:
+	mutex_unlock(&priv->shared_lock);
+	return ret;
+}
+
+/**
+ * phy_mode_check() - check if mode names the PHY interface.
+ * @mode: PHY interface mode string
+ * @interface: PHY interface mode to compare against
+ *
+ * Return: true if @mode matches @interface, false otherwise.
+ */
+static bool phy_mode_check(const char *mode, int interface)
+{
+	return !strcasecmp(mode, phy_modes(interface));
+}
+
+/**
+ * yt8824_phy_package_probe_once()  - init phy package for phy8824.
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_phy_package_probe_once(struct phy_device *phydev)
+{
+	struct yt8824_shared_priv *priv = phy_package_get_priv(phydev);
+	struct device_node *np = phy_package_get_node(phydev);
+	const char *mode;
+
+	if (!priv || !np) {
+		phydev_err(phydev, "%s: missing shared priv or DT node\n",
+			   __func__);
+		return -EINVAL;
+	}
+	/* Initialise shared lock for YT8824 */
+	mutex_init(&priv->shared_lock);
+	priv->package_mode = PHY_INTERFACE_MODE_INTERNAL;
+	if (!of_property_read_string(np, "motorcomm,package-mode",
+				     &mode)) {
+		if (phy_mode_check(mode, PHY_INTERFACE_MODE_INTERNAL)) {
+			priv->package_mode = PHY_INTERFACE_MODE_INTERNAL;
+		} else if (phy_mode_check(mode,
+					  PHY_INTERFACE_MODE_10G_QXGMII)) {
+			priv->package_mode = PHY_INTERFACE_MODE_10G_QXGMII;
+		} else {
+			phydev_err(phydev, "unsupported package-mode %s\n",
+				   mode);
+			return -EINVAL;
+		}
+	} else {
+		phydev_err(phydev, "missing package-mode in PHY package node.\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/**
+ * yt8824_probe() - phy8824 probe.
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_probe(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	struct yt8824_shared_priv *shared_priv;
+	int ret;
+
+	ret = devm_of_phy_package_join(dev, phydev, sizeof(*shared_priv));
+	if (ret)
+		return ret;
+
+	/* Ensure other PHY probes wait for shared mutex initialization. */
+	phy_package_lock(phydev);
+	if (phy_package_probe_once(phydev)) {
+		ret = yt8824_phy_package_probe_once(phydev);
+		if (ret) {
+			phy_package_unlock(phydev);
+			return ret;
+		}
+	}
+	phy_package_unlock(phydev);
+
+	return 0;
+}
+
 static struct phy_driver motorcomm_phy_drvs[] = {
 	{
 		PHY_ID_MATCH_EXACT(PHY_ID_YT8511),
@@ -3190,13 +5056,29 @@ static struct phy_driver motorcomm_phy_drvs[] = {
 		.suspend		= yt8821_suspend,
 		.resume			= yt8821_resume,
 	},
+	{
+		PHY_ID_MATCH_EXACT(PHY_ID_YT8824),
+		.name = "YT8824 Quad Ports 2.5Gbps Ethernet",
+		.get_features = yt8824_get_features,
+		.read_page = yt8824_read_page,
+		.write_page = yt8824_write_page,
+		.probe = yt8824_probe,
+		.config_aneg = yt8824_config_aneg,
+		.aneg_done = yt8824_aneg_done,
+		.config_init = yt8824_config_init,
+		.read_status = yt8824_read_status,
+		.soft_reset = yt8824_soft_reset,
+		.suspend = yt8824_suspend,
+		.resume = yt8824_resume,
+	},
 };
 
 module_phy_driver(motorcomm_phy_drvs);
 
-MODULE_DESCRIPTION("Motorcomm 8511/8521/8531/8531S/8821 PHY driver");
+MODULE_DESCRIPTION("Motorcomm 8511/8521/8531/8531S/8821/8824 PHY driver");
 MODULE_AUTHOR("Peter Geis");
 MODULE_AUTHOR("Frank");
+MODULE_AUTHOR("Kyle");
 MODULE_LICENSE("GPL");
 
 static const struct mdio_device_id __maybe_unused motorcomm_tbl[] = {
@@ -3206,6 +5088,7 @@ static const struct mdio_device_id __maybe_unused motorcomm_tbl[] = {
 	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8531) },
 	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8531S) },
 	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8821) },
+	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8824) },
 	{ /* sentinel */ }
 };
 
-- 
2.25.1

Re: [PATCH net-next v23 3/3] net: phy: Add driver for Motorcomm Quad 2.5GbE phy

[Jakub Kicinski]

On Thu,  1 Oct 2026 03:45:13 +0800 Kyle Switch wrote:
> +	/* 100M template amplitude */
> +	ctrl = FIELD_PREP(YT8824_DAC_IMID_CH3_ORG, 0x45);
> +	ctrl |= FIELD_PREP(YT8824_DAC_IMID_CH2_ORG, 0x45);
> +	mask = YT8824_DAC_IMID_CH3_ORG | YT8824_DAC_IMID_CH3_ORG;

cocci says:

drivers/net/phy/motorcomm.c:1372:8-31: duplicated argument to & or |

Please also wait for the AI review to be published before reposting.
-- 
pw-bot: cr

Re: [PATCH net-next v23 1/3] dt-bindings: net: Document Motorcomm YT8824 PHY package

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In motorcomm,yt8824.yaml, the rules for the package `reg` exist
  only as prose, and the schema does not enforce them.

--

dt-bindings: net: Document Motorcomm YT8824 PHY package

This commit adds a new DT binding schema for the Motorcomm YT8824 quad
PHY package. The schema is based on ethernet-phy-package.yaml, and a
vendor property, motorcomm,package-mode, selects internal or 10g-qxgmii
mode. The commit also adds the new file and a co-maintainer to the
MOTORCOMM PHY DRIVER entry in MAINTAINERS.

> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml
> new file mode 100644
> index 0000000000000..4c67f8afcf27a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml
> @@ -0,0 +1,78 @@

[ ... ]

> +  reg:
> +    description:
> +      The absolute MDIO address of the YT8824 shared top extend
> +      register block.
> +      For internal mode, this is fixed at 9;
> +      For external mode, it is the base MDIO address of the four
> +      member PHYs plus 4, where base is typically 0x0.
> +      Note that this is NOT the reg of the first member PHY.

[Severity: Low]
The rules for reg are only in the description text. No allOf/if-then
block ties reg to motorcomm,package-mode, so this node would pass
dt_binding_check:

    motorcomm,package-mode = "internal";
    reg = <4>;

That passes even though the description says reg is fixed at 9 in
internal mode. Could an if/then be added that requires reg to be 9 when
motorcomm,package-mode is "internal"?

The value matters at runtime, because of_phy_package_join() uses the
package reg as base_addr:

drivers/net/phy/phy_package.c:of_phy_package_join() {
    ...
	if (of_property_read_u32(package_node, "reg", &base_addr)) {
		ret = -EINVAL;
		goto exit;
	}

	ret = phy_package_join(phydev, base_addr, priv_size);
    ...
}

The driver added later in the series ("net: phy: Add driver for
Motorcomm Quad 2.5GbE phy") has ytphy_read_top_ext() and
ytphy_write_top_ext(). Both access the shared top block through
__phy_package_read() and __phy_package_write() with offset 0, so they go
to exactly that address.

The inherited ethernet-phy-package.yaml describes reg as "Commonly the
ID of the first PHY in the PHY package". That is the value this binding
says not to use.

If a board DT sets reg to the first member PHY's address, would the
top-block page select and data writes end up in that member PHY's
register space?

The two modes also seem to use different address layouts. In the
internal-mode example below, the member PHYs are at 4-7 and the top
block is at 9. That is base plus 5, with 8 unused. External mode is
described as base plus 4.

Is that difference real in the hardware? If so, could the description
say so explicitly?

[ ... ]

> +        ethernet-phy-package@9 {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            compatible = "motorcomm,yt8824-package";
> +            reg = <9>;
> +
> +            motorcomm,package-mode = "internal";
> +
> +            ethernet-phy@4 {
> +                reg = <4>;
> +            };

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930194513.663575-1-kyle.switch%40motor-comm.com

Re: [PATCH net-next v23 2/3] net: phy: Add support for Template Control register for PMA

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The patch adds MDIO_PMA_10GBT_TESTMODE_MASK to the exported uAPI
  header include/uapi/linux/mdio.h as GENMASK(15, 13).
- [Low] phy: genphy_c45_template_testmode returns -EOPNOTSUPP instead of
  -EINVAL
- [Low] phy: vendor-specific 'Template' terminology pollutes generic API

--

net: phy: Add support for Template Control register for PMA

This commit adds the genphy_c45_template_testmode() helper. The helper
writes the 10GBASE-T PMA test mode field in register 132. The commit
also adds the register and field definitions to the uapi mdio.h header.

> diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
> index 06f4bc3c20c71..353ca13c64fca 100644
> --- a/include/uapi/linux/mdio.h
> +++ b/include/uapi/linux/mdio.h

[ ... ]

> @@ -320,6 +321,17 @@
>  /* PMA 10GBASE-R Fast Retrain status and control register. */
>  #define MDIO_PMA_10GBR_FSRT_ENABLE	0x0001	/* Fast retrain enable */
>  
> +/* PMA 10GBASE-T Template Test Mode Register*/
> +#define MDIO_PMA_10GBT_TESTMODE_MASK GENMASK(15, 13) /* Template test mode */

[Severity: Low]
Will this build in userspace? include/uapi/linux/mdio.h is an exported
header, and GENMASK() isn't defined for userspace.

The uapi header only includes these:

include/uapi/linux/mdio.h:
    #include <linux/types.h>
    #include <linux/mii.h>

Neither of them, nor anything they pull in (ethtool.h, const.h,
if_ether.h, ...), defines GENMASK(). The uapi <linux/bits.h> only
provides __GENMASK(), __GENMASK_ULL() and __GENMASK_U128().

In the kernel this only works because the internal wrapper includes
bitfield.h right after the uapi header:

include/linux/mdio.h:
    #include <uapi/linux/mdio.h>
    #include <linux/bitfield.h>

So a userspace program that includes <linux/mdio.h> and uses
MDIO_PMA_10GBT_TESTMODE_MASK would fail to compile with an undefined
GENMASK.

This file already has the same problem in MDIO_AN_C73_0_S_MASK,
MDIO_AN_C73_0_E_MASK and MDIO_AN_C73_1_T_MASK. This patch adds one more
exported constant with it.

Could this use a literal (0xe000) or a uapi-safe form such as
__GENMASK()? Another option is to move the mask into
include/linux/mdio.h.

> +#define MDIO_PMA_10GBT_TESTMODE_NORMAL 0x0 /* Template Normal */
> +#define MDIO_PMA_10GBT_TESTMODE_1 0x1 /* Template TestMode1 */

[ ... ]

> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> --- a/drivers/net/phy/phy-c45.c
> +++ b/drivers/net/phy/phy-c45.c
> @@ -1408,6 +1408,29 @@ int genphy_c45_fast_retrain(struct phy_device *phydev, bool enable)
>  }
>  EXPORT_SYMBOL_GPL(genphy_c45_fast_retrain);
>  
> +/**
> + * genphy_c45_template_testmode - configure template testmode registers
> + * @phydev: target phy_device struct
> + * @test_mode: testmode includes Normal to Test mode 7
> + *
> + * Description: Set template testmode include Normal to Test mode 7
> + *
> + * Return: 0 on success, or a negative error code on failure (e.g. register
> + * read/write error).
> + */
> +int genphy_c45_template_testmode(struct phy_device *phydev, u16 test_mode)
> +{
> +	u16 ctrl;
> +
> +	if (test_mode > MDIO_PMA_10GBT_TESTMODE_7)
> +		return -EOPNOTSUPP;
> +
> +	ctrl = FIELD_PREP(MDIO_PMA_10GBT_TESTMODE_MASK, test_mode);
> +	return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_10GBT_TESTMODE,
> +			      MDIO_PMA_10GBT_TESTMODE_MASK, ctrl);
> +}
> +EXPORT_SYMBOL_GPL(genphy_c45_template_testmode);
> +
>  /**
>   * genphy_c45_plca_get_cfg - get PLCA configuration from standard registers
>   * @phydev: target phy_device struct

[Severity: Low]
Where does the term "Template" come from? IEEE 802.3 calls register
1.132 the 10GBASE-T test mode register, and bits 15:13 are the test
mode control field. "Template" is not part of either name. Is it taken
from the Motorcomm datasheet?

The genphy_c45_* helpers and the uapi mdio.h definitions are generic
and should follow the naming in the standard. A vendor term here could
make other driver authors think the standard has a separate "template"
test mode.

The new defines already leave the term out
(MDIO_PMA_10GBT_TESTMODE, MDIO_PMA_10GBT_TESTMODE_MASK). Only the
function name, its kernel-doc and the uapi section comment use it,
which is also inconsistent.

Would a name like genphy_c45_pma_10gbt_test_mode() work for you? The
kernel-doc, the mdio.h comment ("PMA 10GBASE-T test mode register")
and the commit subject could be updated to match.

[Severity: Low]
Is -EOPNOTSUPP the right error code here? A test_mode above
MDIO_PMA_10GBT_TESTMODE_7 means the caller passed a bad argument. It
doesn't mean the operation is unsupported, so -EINVAL seems to fit
better.

genphy_c45_template_testmode() is an exported genphy helper, and PHY
drivers will usually return its value unchanged to their caller.
By convention, -EOPNOTSUPP means the PHY doesn't implement the
operation. Code further up may then report the feature as unsupported
or fall back to something else. It would not tell the user that the
requested mode was out of range.

Could the out-of-range case return -EINVAL instead? -EOPNOTSUPP could
then be kept for a PHY that really lacks register 1.132, if you plan
to check for that.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930194513.663575-1-kyle.switch%40motor-comm.com

Re: [PATCH net-next v23 3/3] net: phy: Add driver for Motorcomm Quad 2.5GbE phy

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 7 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 3 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] In yt8824_config_utp_init_paged() (drivers/net/phy/motorcomm.c),
  U0 TX channels CH0-CH3 are put into reset and never released.
- [High] net: phy: motorcomm: missing phy_package_init_once in
  yt8824_config_init disrupts active ports
- [Medium] In yt8824_config_utp_init_paged(), the '100M template
  amplitude' write to YT8824_DAC_IMID_CH2_CH3_CFG_ORG_REG (0x46e) has a…
- [Medium] yt8824_read_status_paged() sets phydev->speed = SPEED_UNKNOWN
  and phydev->duplex = DUPLEX_UNKNOWN whenever the link is down (lines…
- [Medium] yt8824_sds_isolate_and_softreset_paged() (lines 1060-1078) and
  yt8824_extern_config_sds_init_paged() hold the bus-wide MDIO mutex…
- [Low] yt8824_read_status_paged() calls genphy_c45_read_lpa() only when
  autoneg is enabled and complete (line 4602).
- [Low] In yt8824_probe(), phy_package_probe_once() sets the package
  'probed' bit through test_and_set_bit() before…

--

net: phy: Add driver for Motorcomm Quad 2.5GbE phy

This adds support for the Motorcomm YT8824 quad-port 2.5G PHY to the
existing motorcomm driver. The driver uses the phy_package helpers to
reach the shared top extended register space. A package-wide mutex
serializes the UTP/SerDes space switching between the four ports.

> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
> index 90a4f86f2758b..dfa0612d713cb 100644
> --- a/drivers/net/phy/motorcomm.c
> +++ b/drivers/net/phy/motorcomm.c

[ ... ]

> @@ -626,11 +935,1125 @@ static int ytphy_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)

[ ... ]

> +static int yt8824_sds_isolate_and_softreset_paged(struct phy_device *phydev)
> +{
> +	int old_page = YT8824_RSSR_UTP_SPACE;
> +	int val = 0;
> +	int ret = -1;
> +
> +	old_page = phy_select_page(phydev, YT8824_RSSR_SERDES_SPACE);
> +	if (old_page < 0)
> +		goto err_restore_page;
> +
> +	/* sds softreset and disable isolate */
> +	ret = __phy_modify(phydev, MII_BMCR, BMCR_RESET | BMCR_ISOLATE,
> +			   BMCR_RESET & ~BMCR_ISOLATE);
> +	if (ret < 0)
> +		goto err_restore_page;
> +
> +	/* poll while still holding the lock */
> +	ret = read_poll_timeout(__phy_read, val,
> +				(val < 0) || !(val & BMCR_RESET), 50000, 600000,
> +				true, phydev, MII_BMCR);

[Severity: Medium]
phy_select_page() takes the bus-wide mii_bus->mdio_lock and
phy_restore_page() releases it. That means this read_poll_timeout() sleeps
with the bus lock held. Because sleep_before_read is true, each call holds
it for at least 50ms and up to 600ms.

Is it ok to hold the global MDIO lock that long?

In 10G-QXGMII mode this path runs from soft_reset, suspend, resume and the
error restore paths, for each of the four ports. phy_init_hw() calls
.soft_reset and then .config_init, and yt8824_config_init() calls
yt8824_soft_reset() again. So a single port init goes through several of
these windows.

yt8824_extern_config_sds_init_paged() does the same thing. It holds the lock
across the whole SerDes init sequence and then runs another sleeping
BMCR_RESET poll at the end.

While the lock is held, the sibling ports' phy_state_machine() work, PHYs in
other packages and any switch on the same bus are all blocked. That can in
turn stall ethtool/RTNL users waiting on those phydev locks.

phy_read_poll_timeout() drops the bus lock between polls to avoid this. Here
that is harder, because the UTP/SerDes space select lives in the
package-common top-ext register.

> +	if (val < 0)
> +		ret = val;
> +
> +err_restore_page:
> +	return phy_restore_page(phydev, old_page, ret);
> +}

[ ... ]

> +static int yt8824_config_utp_init_paged(struct phy_device *phydev)
> +{

[ ... ]

> +	/* 100M template amplitude */
> +	ctrl = FIELD_PREP(YT8824_DAC_IMID_CH3_ORG, 0x45);
> +	ctrl |= FIELD_PREP(YT8824_DAC_IMID_CH2_ORG, 0x45);
> +	mask = YT8824_DAC_IMID_CH3_ORG | YT8824_DAC_IMID_CH3_ORG;
                                          ^^^^

[Severity: Medium]
Should the second term be YT8824_DAC_IMID_CH2_ORG?

ytphy_modify_ext() goes through __phy_modify(), which writes
(old & ~mask) | set. With CH2 left out of the mask, the CH2 field of
YT8824_DAC_IMID_CH2_CH3_CFG_ORG_REG becomes old | 0x45 instead of 0x45.

The three sibling writes to 0x46f, 0x470 and 0x471 include both fields in the
mask.

> +	ret = ytphy_modify_ext_with_lock(phydev,
> +					 YT8824_DAC_IMID_CH2_CH3_CFG_ORG_REG,
> +					 mask, ctrl);
> +	if (ret < 0)
> +		goto err_restore;

[ ... ]

> +	ctrl = FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH0, 0);
> +	ctrl |= FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH1, 0);
> +	ctrl |= FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH2, 0);
> +	ctrl |= FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH3, 0);
> +	mask  = YT8824_U0_CSR_RESETB_TX_CH0 | YT8824_U0_CSR_RESETB_TX_CH1 |
> +		YT8824_U0_CSR_RESETB_TX_CH2 | YT8824_U0_CSR_RESETB_TX_CH3;
> +	ret = ytphy_modify_ext_with_lock(phydev, YT8824_U0_CRS_RESET_REG,
> +					 mask, ctrl);
> +	if (ret < 0)
> +		goto err_restore;

[Severity: High]
Is the SerDes and PLL setup done here really per-port?

yt8824_config_init() runs the whole SerDes sequence in
yt8824_extern_config_sds_init_paged() and the UTP sequence in
yt8824_config_utp_init_paged() for every PHY in the package. It does
this each time phy_init_hw() is called for that PHY:

phy_attach_direct() / mdio_bus_phy_resume()
  phy_init_hw()
    yt8824_soft_reset()
    yt8824_config_init()
      yt8824_extern_config_sds_init_paged()
      yt8824_config_utp_init_paged()

In 10G-QXGMII mode all four ports are multiplexed onto one SerDes
lane, which is also why the init sets
YT8824_CSR_AFE_MANUAL_CTRL_10P3125G.

yt8824_extern_config_sds_init_paged() rewrites the SerDes PLL, LDO and
training registers. It restarts calibration through
YT8824_RESTART_CAL_CTRL_REG and ends with a BMCR_RESET on the SerDes.

yt8824_config_utp_init_paged() toggles YT8824_PLL_DAC_RST. It also
drives the TX RESETB bits in YT8824_U0_CRS_RESET_REG and
YT8824_U1_CRS_RESET_REG low before releasing them.

If any of these registers are package-wide rather than per-port, then
attaching or resuming one port resets the shared lane, PLL and TX
paths. That happens underneath sibling ports that already have link,
so their traffic drops until the lane retrains. Many MAC drivers
connect the PHY from ndo_open, so a plain ifup on one port is enough to
trigger this.

phy_package_init_once() exists for this case. Other multi-port PHY
drivers such as mscc and qca807x use it so that only the first port
runs the package-global setup.

Could the global part be split into a helper guarded by
phy_package_init_once(), leaving only the per-port writes in
yt8824_config_init()? Which of the registers touched here are actually
shared between the four ports?

If the shared SerDes and PLL must be reprogrammed after the package
loses power across suspend, how should that work with the once bit?

[ ... ]

> +	ctrl = FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH0, 1);
> +	ctrl |= FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH1, 1);
> +	ctrl |= FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH2, 1);
> +	ctrl |= FIELD_PREP(YT8824_U0_CSR_RESETB_TX_CH3, 1);
> +	mask = YT8824_U0_CSR_RESETB_TX_CH0 | YT8824_U0_CSR_RESETB_TX_CH1 |
> +	       YT8824_U0_CSR_RESETB_TX_CH2 | YT8824_U0_CSR_RESETB_TX_CH3;
> +
> +	ctrl = FIELD_PREP(YT8824_U1_CSR_RESETB_TX_CH0, 1);

[Severity: High]
Is a ytphy_modify_ext_with_lock() call on YT8824_U0_CRS_RESET_REG missing
here?

Earlier in yt8824_config_utp_init_paged(), the active-low U0 TX RESETB bits
are written to 0. The U0 release value and mask are built here, but ctrl and
mask are overwritten by the U1 values straight away. Only
YT8824_U1_CRS_RESET_REG is written afterwards.

This means U0 TX channels CH0-CH3 seem to stay in reset, while the function
still returns 0.

This path runs on every attach and resume, in both package modes:

phy_init_hw()
  yt8824_config_init()
    yt8824_config_utp_init_paged()

> +	ctrl |= FIELD_PREP(YT8824_U1_CSR_RESETB_TX_CH1, 1);
> +	ctrl |= FIELD_PREP(YT8824_U1_CSR_RESETB_TX_CH2, 1);
> +	ctrl |= FIELD_PREP(YT8824_U1_CSR_RESETB_TX_CH3, 1);
> +	mask = YT8824_U1_CSR_RESETB_TX_CH0 | YT8824_U1_CSR_RESETB_TX_CH1 |
> +	       YT8824_U1_CSR_RESETB_TX_CH2 | YT8824_U1_CSR_RESETB_TX_CH3;
> +	ret = ytphy_modify_ext_with_lock(phydev, YT8824_U1_CRS_RESET_REG,
> +					 mask, ctrl);

[ ... ]

> @@ -3104,6 +4527,449 @@ static int yt8821_resume(struct phy_device *phydev)

[ ... ]

> +static int yt8824_read_status_paged(struct phy_device *phydev)
> +{
> +	int ret;
> +	int val;
> +
> +	ret = phy8824_page_write_with_lock(phydev, YT8824_RSSR_UTP_SPACE);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = genphy_read_status(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
> +		ret = genphy_c45_read_lpa(phydev);
> +		if (ret < 0)
> +			return ret;
> +	}

[Severity: Low]
When autoneg is enabled but not complete (link down or renegotiation
pending), genphy_c45_read_lpa() is skipped. genphy_read_status() then calls
genphy_read_lpa(), which only clears the clause 22 partner bits:

drivers/net/phy/phy_device.c:genphy_read_lpa() {
    ...
	if (phydev->autoneg == AUTONEG_ENABLE) {
		if (!phydev->autoneg_complete) {
			mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising,
							0);
			mii_lpa_mod_linkmode_lpa_t(phydev->lp_advertising, 0);
			return 0;
		}
    ...
}

Could a 2500baseT_Full bit from an earlier 2.5G negotiation stay in
phydev->lp_advertising and keep showing up in ethtool?

Other 2.5G drivers such as rtl822x_read_status() clear it explicitly with
mii_10gbt_stat_mod_linkmode_lpa_t(phydev->lp_advertising, 0). The existing
yt8821_read_status() has the same pattern.

> +
> +	if (!phydev->link) {
> +		phydev->speed = SPEED_UNKNOWN;
> +		phydev->duplex = DUPLEX_UNKNOWN;

[Severity: Medium]
Does this lose a forced speed/duplex when autoneg is disabled?

With AUTONEG_DISABLE, phylib keeps the user's forced setting only in
phydev->speed and phydev->duplex. genphy_read_status() has just restored
them from BMCR via genphy_read_status_fixed(). After one link-down poll they
are replaced with SPEED_UNKNOWN and DUPLEX_UNKNOWN.

On the next ifdown/ifup or resume:

phy_start() / mdio_bus_phy_resume()
  _phy_start_aneg()
    phy_sanitize_settings()
      phy_caps_lookup(SPEED_UNKNOWN, DUPLEX_UNKNOWN, supported, false)

No capability matches speed -1, so phy_caps_lookup() returns the slowest
supported mode. Then yt8824_config_aneg()->__genphy_config_aneg() takes the
speed < SPEED_1000 path, and genphy_setup_forced() programs 10Mb/s instead
of the forced 100/1000/2500.

genphy_read_status() and yt8821_read_status() leave the forced values alone
in this case.

> +		if (phydev->autoneg == AUTONEG_ENABLE)
> +			phy_resolve_aneg_pause(phydev);
> +		return 0;
> +	}

[ ... ]

> +static int yt8824_probe(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct yt8824_shared_priv *shared_priv;
> +	int ret;
> +
> +	ret = devm_of_phy_package_join(dev, phydev, sizeof(*shared_priv));
> +	if (ret)
> +		return ret;
> +
> +	/* Ensure other PHY probes wait for shared mutex initialization. */
> +	phy_package_lock(phydev);
> +	if (phy_package_probe_once(phydev)) {
> +		ret = yt8824_phy_package_probe_once(phydev);
> +		if (ret) {
> +			phy_package_unlock(phydev);
> +			return ret;
> +		}
> +	}
> +	phy_package_unlock(phydev);

[Severity: Low]
This may not be reachable in practice. phy_package_probe_once() sets the
package once bit with test_and_set_bit() before the DT parsing in
yt8824_phy_package_probe_once(), which can fail. The bit is not cleared
again on failure.

If another package member had already joined at that point, it would skip
the init and probe successfully with the default PHY_INTERFACE_MODE_INTERNAL.
It would not fail on the missing or unsupported motorcomm,package-mode.

However, phy_driver_register() sets PROBE_FORCE_SYNCHRONOUS. With
sequential probing, the failing PHY's devres release calls
phy_package_leave(), the refcount drops to zero and the shared struct is
freed, once bit included. The next joiner then starts fresh.

The !priv || !np check in yt8824_phy_package_probe_once() also looks
unreachable. of_phy_package_join() always sets shared->np, and priv is
allocated because priv_size is non-zero.

Would it be cleaner to validate the DT property before consuming the once
bit, or is the current ordering fine given synchronous probing?

> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930194513.663575-1-kyle.switch%40motor-comm.com