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

[PATCH net-next v27 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 v27:
1) Remove unuse macros and "," after yt8824_read_page() and
   yt8824_write_page().
2) Modify the function description for yt8824_restore_working_status()
   and yt8824_config_init().
3) Add UTP power up operation after init success in yt8824_config_init().
4) Set sds_initialized to 1 in resume.

changes in v26:
1) Rebase

changes in v25:
1) Correct the naming of some register name macro definitions used for
phy8824 initialization to follow the conventional naming style.

No functional change.

changes in v24:
1) Fix the typo during sds init.
2) Add priv->sds_initialized to indicate whether the SDS extended
registers shared by the four channels have been initialized.

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                       |    4 +-
 drivers/net/phy/motorcomm.c                   | 1926 ++++++++++++++++-
 drivers/net/phy/phy-c45.c                     |   23 +
 include/linux/phy.h                           |    1 +
 include/uapi/linux/mdio.h                     |   12 +
 7 files changed, 2043 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml

-- 
2.25.1

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

[Kyle Switch]

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

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
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 23e9194483d9..ac8e12e7b839 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18424,8 +18424,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 v27 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 a5a419bc400e..81cb7f15bb72 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -2359,6 +2359,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 v27 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.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Kyle Switch <kyle.switch@motor-comm.com>
---
 drivers/net/phy/Kconfig     |    4 +-
 drivers/net/phy/motorcomm.c | 1926 ++++++++++++++++++++++++++++++++++-
 2 files changed, 1927 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 30e9d22c8d1a..04593a770e43 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -361,10 +361,12 @@ 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, YT861x PHYs
-	  integrated in YT921x switches, and YT8821 2.5Gbps PHYs.
+	  integrated in YT921x switches, YT8821 2.5Gbps PHYs and YT8824 4 * 2.5G
+	  PHYs.
 
 config NATIONAL_PHY
 	tristate "National Semiconductor PHYs"
diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 463e1ea3efc9..6628ebcca081 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -1,19 +1,25 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Motorcomm 8511/8521/8522/8531/8531S/861x/8821 PHY driver.
+ * Motorcomm 8511/8521/8522/8531/8531S/861x/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/ethtool_netlink.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
@@ -22,6 +28,7 @@
 /* Live captured from YT921x switch internal MDIO bus */
 #define PHY_ID_INT861X		0x01e04281
 #define PHY_ID_YT8821		0x4f51ea19
+#define PHY_ID_YT8824		0x4f51e8b8
 /* YT8521/YT8531S/YT8821 Register Overview
  *	UTP Register space	|	FIBER Register space
  *  ------------------------------------------------------------
@@ -33,6 +40,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 */
@@ -397,6 +416,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_ENABLE_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_RETRAIN_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_SAVE			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_THR_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_SLAVE_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_EC_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_THR			GENMASK(15, 8)
+#define YT8824_LFER_FAIL_THR			GENMASK(7, 0)
+#define YT8824_FR_REQ_REG			0x00be
+#define YT8824_FR_REQ_THR			GENMASK(4, 0)
+#define YT8824_GN_MU_COARSE_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_CSR_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_CSR_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_THR_LNG			GENMASK(6, 0)
+#define YT8824_TRAINING_CTRL_REG		0x0492
+#define YT8824_VDAC_AMP_THR_MED			GENMASK(14, 8)
+#define YT8824_VDAC_AMP_THR_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_THR			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_THR			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
@@ -415,6 +654,14 @@ 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;
+	/* true once the package-wide SERDES init has been done */
+	bool sds_initialized;
+};
+
 /**
  * ytphy_read_ext() - read a PHY's extended register
  * @phydev: a pointer to a &struct phy_device
@@ -453,6 +700,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
@@ -649,6 +960,1156 @@ 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_ENABLE_ODD_PREAMBLE, 3);
+	mask = YT8824_U0_U1_ENABLE_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_RETRAIN_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_SAVE, 0x83);
+	mask = YT8824_POWER_SAVE;
+	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_CH2_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_THR_S, 0x36);
+	mask = YT8824_CABLE_VSHT_THR_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_SLAVE_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_EC_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_THR, 0xFF);
+	ctrl |= FIELD_PREP(YT8824_LFER_FAIL_THR, 0xFF);
+	mask = YT8824_LDPC_FAIL_THR | YT8824_LFER_FAIL_THR;
+	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_THR, 0x06);
+	mask = YT8824_FR_REQ_THR;
+	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_COARSE_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_CSR_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_CSR_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;
+	ret = ytphy_modify_ext_with_lock(phydev, YT8824_U0_CSR_RESET_REG,
+					 mask, ctrl);
+	if (ret < 0)
+		goto err_restore;
+
+	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_CSR_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_sds_softreset() - config external phy8824 sds softrest
+ * @phydev: target phy_device struct
+ *
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8824_extern_sds_softreset(struct phy_device *phydev)
+{
+	int old_page = YT8824_RSSR_UTP_SPACE;
+	int ret = -1;
+	int val = 0;
+
+	old_page = phy_select_page(phydev, YT8824_RSSR_SERDES_SPACE);
+	if (old_page < 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_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;
+
+	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_THR_LNG, 0x7F);
+	mask = YT8824_VDAC_AMP_THR_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_THR_MED, 0x7F);
+	ctrl |= FIELD_PREP(YT8824_VDAC_AMP_THR_SHT, 0x7F);
+	mask = YT8824_VDAC_AMP_THR_MED | YT8824_VDAC_AMP_THR_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_THR, 0x0000);
+	mask = YT8824_EOD_ERR_CNT_THR;
+	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_THR, 0x7F);
+	mask = YT8824_DFE_VDAC_CODE_THR;
+	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_THR_MED, 0x7F);
+	ctrl |= FIELD_PREP(YT8824_VDAC_AMP_THR_SHT, 0xFF);
+	mask = YT8824_VDAC_AMP_THR_MED | YT8824_VDAC_AMP_THR_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_THR_MED, 0x7F);
+	ctrl |= FIELD_PREP(YT8824_VDAC_AMP_THR_SHT, 0x7F);
+	mask = YT8824_VDAC_AMP_THR_MED | YT8824_VDAC_AMP_THR_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;
+
+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 {
+		if (!priv->sds_initialized) {
+			ret = yt8824_extern_config_sds_init_paged(phydev);
+			if (ret < 0)
+				goto err;
+			priv->sds_initialized = true;
+		}
+		/*
+		 * Configuring the MII registers of the SDS,
+		 * which are private to each channel.
+		 */
+		ret = yt8824_extern_sds_softreset(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)
 {
@@ -3240,6 +4701,450 @@ 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->sds_initialized = false;
+	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),
@@ -3337,13 +5242,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/861x/8821 PHY driver");
+MODULE_DESCRIPTION("Motorcomm 8511/8521/8531/8531S/861x/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[] = {
@@ -3354,6 +5275,7 @@ static const struct mdio_device_id __maybe_unused motorcomm_tbl[] = {
 	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8531S) },
 	{ PHY_ID_MATCH_EXACT(PHY_ID_INT861X) },
 	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8821) },
+	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8824) },
 	{ /* sentinel */ }
 };
 
-- 
2.25.1

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

[Jakub Kicinski]

On Tue, 20 Oct 2026 10:19:26 +0800 Kyle Switch wrote:
> 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.

## Form letter - net-next-closed

The merge window for Linux v7.4 has begun, and the net-next
tree is now closed for new drivers, features, code refactoring
and optimizations. We are currently accepting bug fixes only.

Please repost when net-next reopens. There is some uncertainty
about how long this merge window will last, so please check
https://netdev.bots.linux.dev/net-next.html
before reposting. net-next _may_ reopen on Nov 2nd.

See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
-- 
pw-bot: defer
pv-bot: closed

Re: [PATCH net-next v27 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 Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml,
  the package 'reg' is redefined as the absolute MDIO address of the…

--

dt-bindings: net: Document Motorcomm YT8824 PHY package

This commit adds a DT binding schema for the Motorcomm YT8824, a package
of four PHYs, built on ethernet-phy-package.yaml. It adds a
motorcomm,package-mode property and defines the package reg as the MDIO
address of the shared top extend register block. It also adds the new
file and a second 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 000000000000..4c67f8afcf27
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8824.yaml

[ ... ]

> +  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

[Severity: Low]
Does the schema enforce "fixed at 9" for internal mode anywhere? I could
not find an allOf/if-then on motorcomm,package-mode that sets reg to
const 9. So a node with package-mode = "internal" and any reg in the
0..31 range inherited from ethernet-phy-package.yaml would still pass
dt_binding_check.

The two address rules also don't line up. The internal mode example below
has the member PHYs at 4..7 and reg = <9>:

    ethernet-phy-package@9 {
        ...
        reg = <9>;

        motorcomm,package-mode = "internal";

        ethernet-phy@4 {

For that layout, the "base plus 4" rule from external mode would give 8.
Could the description explain why the top extend block sits at a different
offset in internal mode?

The reg value matters at runtime. Later in this series,
of_phy_package_join() in drivers/net/phy/phy_package.c reads the package
node's reg into base_addr:

    if (of_property_read_u32(package_node, "reg", &base_addr)) {

and ytphy_read_top_ext()/ytphy_write_top_ext() in drivers/net/phy/motorcomm.c
access offset 0 from that base:

    ret = __phy_package_write(phydev, 0, YTPHY_PAGE_SELECT, regnum);

With a wrong reg, every top extend access (page select, SerDes PLL control
and so on) would go to a member PHY or to an empty MDIO address.

Would it make sense to add an if/then on motorcomm,package-mode that
constrains reg to const 9 for internal mode?

[ ... ]

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

Re: [PATCH net-next v27 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] include/uapi/linux/mdio.h line 325 defines the new exported uAPI
  macro MDIO_PMA_10GBT_TESTMODE_MASK as GENMASK(15, 13).

--

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

This commit adds definitions for the 10GBASE-T PMA Template Test Mode
register (PMA register 132, bits 15:13) to the mdio uAPI header. It also
adds a genphy_c45_template_testmode() helper, which selects one of the
eight test modes (Normal through TestMode7).

> 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

[ ... ]

> @@ -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 MDIO_PMA_10GBT_TESTMODE_MASK compile when userspace uses it?

GENMASK() is defined in the kernel-only include/linux/bits.h. The
exported include/uapi/linux/bits.h only provides these:

    #define __GENMASK(h, l) ...
    #define __GENMASK_ULL(h, l) ...
    #define __GENMASK_U128(h, l) ...

Also, include/uapi/linux/mdio.h only includes <linux/types.h> and
<linux/mii.h>.

So a userspace program that includes the installed <linux/mdio.h> and
uses MDIO_PMA_10GBT_TESTMODE_MASK would fail with GENMASK undeclared. The
in-tree build works because phy-c45.c gets GENMASK() from the kernel
headers. CONFIG_UAPI_HEADER_TEST would not catch this either, because
the macro is never expanded there.

Could this use __GENMASK(15, 13) with the uapi <linux/bits.h>, or the
literal 0xe000?

The header 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, but this patch adds a new
exported symbol with it. The later patches in the series leave this
definition unchanged.

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

[ ... ]

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

Re: [PATCH net-next v27 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: 0 · Medium: 3 · Low: 4

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).

- [Medium] The commit message is three lines long for a ~1900-line driver
  addition and leaves out behaviour a maintainer needs to judge it.
- [Medium] priv->sds_initialized is only reset in
  yt8824_phy_package_probe_once() and is set to true after the first…
- [Medium] net: phy: motorcomm: yt8824_read_status_paged() clears forced
  speed and duplex
- [Low] yt8824_probe() calls phy_package_probe_once() before
  yt8824_phy_package_probe_once() has validated the package.
- [Low] In yt8824_config_aneg() the forced-2.5G clause sets
  MDIO_AN_10GBT_CTRL_ADV2_5G only if 2500baseT_Full is also in…
- [Low] yt8824_read_status_paged() calls genphy_c45_read_lpa() only when
  autoneg is enabled and autoneg_complete is set.
- [Low] yt8824_sds_isolate_and_softreset_paged() and
  yt8824_extern_sds_softreset() poll BMCR_RESET with…

--

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. It uses the phy_package helpers to access the
shared top extended register space. Two package modes are supported:
internal and 10g-qxgmii.

> 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.

[Severity: Medium]

Could the commit message describe some of the less obvious behaviour
this adds?

yt8824_soft_reset(), yt8824_power_on() and yt8824_power_down() put the
PMA into 10GBASE-T template test mode 1 around every reset or power
transition, and switch it back to normal afterwards. In non-internal
mode they also isolate the SERDES and soft reset it. yt8824_config_init()
reaches the same sequence through yt8824_soft_reset(). The only
explanation in the code is "NOTE: need WA like softreset".

The changelog also doesn't mention the two package modes (internal and
10g-qxgmii). Nor does it mention the one-time package-wide SERDES
calibration, which only runs in 10g-qxgmii mode.

The ethernet-phy-package node must carry the "motorcomm,package-mode"
property, or yt8824_phy_package_probe_once() returns -EINVAL. The
property is documented by "dt-bindings: net: Document Motorcomm YT8824
PHY package" earlier in this series. Could the changelog say that this
property is required?

> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
> index 463e1ea3efc9..6628ebcca081 100644
> --- a/drivers/net/phy/motorcomm.c
> +++ b/drivers/net/phy/motorcomm.c

[ ... ]

> @@ -638,15 +949,1165 @@ 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: Low]

phy_select_page() takes the bus-wide mdio_lock and phy_restore_page()
releases it. sleep_before_read is true, so this poll holds mdio_lock for
at least 50ms and up to 600ms. Is it intended that every other user of
the MDIO bus is blocked for that long? That includes the other YT8824
ports, any other PHYs, and any switch on the bus.

In 10g-qxgmii mode this runs per port from yt8824_soft_reset(), which
phy_init_hw() reaches twice: once as .soft_reset and again through
yt8824_config_init(). It also runs on suspend, on resume and on the
restore path.

yt8824_extern_sds_softreset() has the same poll under the bus lock, and
it runs on every yt8824_config_init().

Could the page be restored and the lock dropped between poll reads
instead?

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

[ ... ]

> +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 {
> +		if (!priv->sds_initialized) {
> +			ret = yt8824_extern_config_sds_init_paged(phydev);
> +			if (ret < 0)
> +				goto err;
> +			priv->sds_initialized = true;
> +		}

[Severity: Medium]

priv->sds_initialized is only cleared in yt8824_phy_package_probe_once().
The shared priv lives as long as any port in the package stays bound.
What happens if the package loses its SERDES configuration after the
flag has been set?

One case is a PHY with reset-gpios that is detached and attached again:

phy_detach()
  phy_device_reset(phydev, 1)
phy_attach_direct()
  phy_init_hw()
    phy_device_reset(phydev, 0)
    yt8824_config_init()

Another is a system suspend that removes PHY power, followed by
mdio_bus_phy_resume()->phy_init_hw().

In both cases yt8824_config_init() would skip
yt8824_extern_config_sds_init_paged(). It would only redo the per-channel
SDS soft reset and the UTP init.

Would the SERDES PLL/CDR/AFE/DFE/TX tuning then stay at reset defaults
in 10g-qxgmii mode until the driver is rebound?

[ ... ]

> @@ -3240,6 +4701,450 @@ 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]

Can the 2500baseT_Full bit in phydev->lp_advertising go stale here?

When the link drops or negotiation restarts, autoneg_complete is 0. In
that case genphy_read_status()->genphy_read_lpa() only clears the clause
22 and 1000BASE-T partner bits:

	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;
	}

genphy_c45_read_lpa() is skipped in that case, so its own
mii_10gbt_stat_mod_linkmode_lpa_t(..., 0) clearing never runs.

Would ethtool then keep reporting a 2.5G link partner capability while
no negotiation is complete? yt8821_read_status() has the same pattern,
and it appears to have been copied here.

[ ... ]

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

[ ... ]

> +	/*
> +	 * 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;

[Severity: Low]

Does this condition match the comment above it? For forced speeds of
1000 and above, __genphy_config_aneg() builds the advertisement from
phydev->supported, not phydev->advertising:

		c = phy_caps_lookup(phydev->speed, phydev->duplex,
				    phydev->supported, true);

Suppose 2.5G is forced after 2500baseT_Full was removed from
phydev->advertising. This clears ADV2_5G, and nothing usable is left in
the advertisement.

This can't be reached today. phy_ethtool_ksettings_set() rejects
AUTONEG_DISABLE with any speed other than 10/100/1000, so the
phydev->speed == SPEED_2500 clause is dead code.

Should that clause be dropped together with the "explicitly forced"
part of the comment? Alternatively, could it stop depending on
phydev->advertising?

[ ... ]

> +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->sds_initialized = false;
> +	priv->package_mode = PHY_INTERFACE_MODE_INTERNAL;
> +	if (!of_property_read_string(np, "motorcomm,package-mode",
> +				     &mode)) {

[ ... ]

> +	} else {
> +		phydev_err(phydev, "missing package-mode in PHY package node.\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

[ ... ]

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

[ ... ]

> +	/* 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;
> +}

[Severity: Low]

What happens to the sibling ports if yt8824_phy_package_probe_once()
fails here?

phy_package_probe_once() does an unconditional test_and_set_bit() on
shared->flags. The INTERNAL default is stored in priv->package_mode
before the property is checked. If "motorcomm,package-mode" is missing
or unsupported, this port fails with -EINVAL, but the failure is not
recorded anywhere.

A sibling PHY in the same package may join before this port's devres
runs phy_package_leave(). Its reference keeps the shared object, and the
once bit, alive. phy_package_probe_once() then returns false for the
sibling, so it skips validation, returns 0, and runs in INTERNAL mode
whatever the DT says.

phylib probes synchronously, so this needs two ports of the package to
probe at the same time. One example is driver_attach() from a module
load racing with mdiobus_register().

Could the validation happen before the once bit is used up? Another
option would be to keep a validated or failed state in the shared priv
for siblings to check.

As a side note, the !priv || !np check can't trigger.
of_phy_package_join() always sets np, and it always allocates priv when
priv_size is non-zero.

[ ... ]

> @@ -3240,6 +4701,450 @@ 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->sds_initialized = false;
> +	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),

[Severity: Medium]
When the link is down, this block overwrites phydev->speed and
phydev->duplex with SPEED_UNKNOWN and DUPLEX_UNKNOWN, whatever
phydev->autoneg is set to. Is that intended for the forced case?

With AUTONEG_DISABLE, phydev->speed and phydev->duplex are more than
status. They also hold the forced mode the user configured.
phy_ethtool_ksettings_get() reports them back. _phy_start_aneg()
passes them to phy_sanitize_settings() before phy_config_aneg().
genphy_read_status() has just refreshed them from BMCR through
genphy_read_status_fixed(), and this block then discards them.

Take "ethtool -s ethX autoneg off speed 100 duplex full" with the
cable unplugged. phy_check_link_status() runs straight after
phy_config_aneg() and loses the forced speed and duplex. Now take the
interface down and up, or suspend and resume. _phy_start_aneg() calls
phy_sanitize_settings(), which calls phy_caps_lookup() with
SPEED_UNKNOWN and exact == false. That lookup falls back to the lowest
supported mode, so the PHY ends up forced to 10/Half rather than the
100/Full the user asked for.

While the link stays down, a partial linkmodes update over ethtool
netlink also starts from these unknown values.
phy_ethtool_ksettings_set() then rejects AUTONEG_DISABLE with an
unknown speed or duplex and returns -EINVAL.

With autoneg enabled, genphy_read_status() already resets speed,
duplex and pause before reading the link partner, so the block looks
redundant there. The phy_resolve_aneg_pause() call is also a no-op.
Duplex has just been set to DUPLEX_UNKNOWN, and
phy_resolve_aneg_pause() only does anything for DUPLEX_FULL.

yt8821_read_status() does not do this. It only calls
yt8821_adjust_status() when the link is up. Could the !phydev->link
branch become a plain return 0, or be limited to AUTONEG_ENABLE?

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