[PATCH net-next v10 0/6] net: pcs: add basic support for RK3568 XPCS

[PATCH net-next v10 0/6] net: pcs: add basic support for RK3568 XPCS

[Coia Prant]

This series adds proper SGMII support for the Rockchip RK3568 SoC
using the integrated Synopsys DesignWare XPCS, along with necessary
fixes and refactoring in the stmmac core and XPCS driver.

Motivation
==========
The RK3568 integrates a DW XPCS accessed via APB3 and connected to
a Naneng Combo SerDes PHY.  Several boards (e.g., Ariaboard
Photonicat) use this interface for Gigabit Ethernet.  However, the
current upstream stmmac driver does not support this configuration,
and the XPCS driver has issues in SGMII poll mode that cause the
link to be reported incorrectly.

This series addresses these issues by:
- Adding ANRESTART support for SGMII link recovery
- Adding a Rockchip XPCS platform glue driver and wiring it up in
  dwmac-rk

Series overview
===============

RK3568 XPCS/SGMII:
  Patch 1: DT binding for Rockchip RK3568 XPCS
  Patch 2: add ANRESTART support for SGMII link recovery
  Patch 3: implement the Rockchip XPCS platform glue driver
  Patch 4: DT binding for Rockchip DWMAC PCS
  Patch 5: wire up SGMII support in dwmac-rk
  Patch 6: update MAINTAINERS

Dependencies
============
- Generic PM domain (genpd) changes:
  https://lore.kernel.org/all/20260925041751.495818-1-coiaprant@gmail.com/
- PHY binding+driver (Naneng Combo PHY SGMII MAC selection):
  https://lore.kernel.org/all/20261005221229.1095843-1-coiaprant@gmail.com/
- stmmac XPCS lifetime management fix:
  https://lore.kernel.org/all/20261005221749.1104430-1-coiaprant@gmail.com/

The DTS changes using the PHY property are sent separately to
Heiko/linux-rockchip. There is no hard build dependency; the pieces
can converge during the merge window.

Changelog
=========

- Resend and split: PHY binding+driver sent to linux-phy, stmmac
  lifetime fix and XPCS binding sent separately, DTS sent to
  linux-rockchip.

Key design decisions
====================
- The stmmac core now delegates XPCS creation entirely to platform
  drivers via pcs_init/pcs_exit.  This is necessary because the
  generic XPCS creation logic would override any XPCS set up by the
  platform driver.

- The Rockchip XPCS driver creates a virtual MDIO bus over the APB3
  registers and implements address remapping.  The generic XPCS core
  handles all PCS configuration via phylink_pcs_ops.

- On RK3568 in SGMII mode, the MAC clock is fixed at 125 MHz and
  cannot be dynamically changed.  In-band mode is used, and the
  generic stmmac set_clk_tx_rate callback is disabled to prevent
  incorrect clock updates that would break RX.

- The SerDes and power domain are attached to the XPCS device tree
  node rather than the MAC node. This reflects the actual hardware
  topology and simplifies the dwmac-rk driver by keeping all PCS-related
  resources self-contained. It also prepares for possible future QSGMII
  support, where a single SerDes serves multiple MACs and would be
  more naturally managed under the XPCS node.

Testing
=======
Board: Ariaboard Photonicat (RK3568)
OS: Armbian (trixie)
Kernel: 6.18 (backports)
Result: The SGMII interface obtains an IP address, SSH works, and
        ping traffic passes without loss.

Notes
=====
- When testing out-band mode with set_clk_tx_rate, only 1000Mbps
  works on both TX/RX; 10/100Mbps only works on TX side.

Acknowledgments
===============
This work was inspired by and builds upon the excellent work of others:
- Serge Semin's Synopsys DesignWare XPCS platform driver (pcs-xpcs-plat.c)
- Clément Léger's Renesas MIIC driver (pcs-rzn1-miic.c)
- The Rockchip TRM and downstream OEM drivers

Thanks in advance,
Coia Prant
---
Coia Prant (6):
  dt-bindings: net: pcs: add rockchip,rk3568-xpcs support
  net: pcs: xpcs: add ANRESTART support for SGMII link recovery
  net: pcs: xpcs: add Rockchip RK3568 platform glue driver
  dt-bindings: net: rockchip-dwmac: document pcs-handle
  net: stmmac: dwmac-rk: add SGMII support for RK3568
  MAINTAINERS: add entry for Rockchip XPCS driver

 .../net/pcs/rockchip,rk3568-xpcs.yaml         | 110 ++++
 .../bindings/net/rockchip-dwmac.yaml          |  18 +
 MAINTAINERS                                   |   9 +
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |   1 +
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 130 +++-
 drivers/net/pcs/Kconfig                       |  25 +
 drivers/net/pcs/Makefile                      |   5 +-
 drivers/net/pcs/pcs-xpcs-rk.c                 | 619 ++++++++++++++++++
 drivers/net/pcs/pcs-xpcs.c                    |  35 +-
 include/linux/pcs/pcs-xpcs-rk.h               |  11 +
 10 files changed, 935 insertions(+), 28 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
 create mode 100644 drivers/net/pcs/pcs-xpcs-rk.c
 create mode 100644 include/linux/pcs/pcs-xpcs-rk.h

-- 
2.47.3

[PATCH net-next v10 1/6] dt-bindings: net: pcs: add rockchip,rk3568-xpcs support

[Coia Prant]

Add device tree binding documentation for the Synopsys DesignWare
XPCS integrated on the Rockchip RK3568 SoC.

The XPCS is accessed over the APB3 bus and internally connected to
a Naneng Combo SerDes PHY.  It supports 1000BASE-X, SGMII, and
QSGMII modes, with four MII ports.

The four MII ports are described as ethernet-pcs-mii@N child nodes,
consumed by the Rockchip XPCS glue driver later in this series.

phys and phy-names are required because dtbs_check only validates
required properties for enabled nodes. The SerDes link is a board-level
design choice (combphy1 on some boards, combphy2 on others), so these
properties must be provided by the board device tree, not the SoC dtsi.

The CRU reset lines (SRST_XPCS*) are intentionally not described: no
in-tree user requests them, and bring-up relies on the PD_PIPE power
domain, the SerDes PHY and the in-IP soft reset. They can be added
later as optional without breaking ABI.

Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 .../net/pcs/rockchip,rk3568-xpcs.yaml         | 110 ++++++++++++++++++
 1 file changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml

diff --git a/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
new file mode 100644
index 0000000000000..703fcff0e3f70
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/pcs/rockchip,rk3568-xpcs.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip RK3568 Synopsys DesignWare Ethernet PCS
+
+maintainers:
+  - Coia Prant <coiaprant@gmail.com>
+
+description: |
+  Rockchip RK3568 SoC integrates a Synopsys DesignWare Ethernet Physical
+  Coding Sublayer (XPCS).
+  The PCS provides an interface between the Media Access Control (MAC)
+  and the Physical Medium Attachment (PMA) sublayer through a Media
+  Independent Interface (GMII).
+
+  The XPCS is accessed over the APB3 bus and internally connected to a
+  Naneng Combo SerDes PHY.
+  It supports 1000BASE-X, SGMII and QSGMII modes.
+
+  The block contains four MII ports that can be individually enabled and
+  routed to one of the Ethernet GMAC controllers via the pcs-handle
+  property in the MAC device tree node.
+
+properties:
+  compatible:
+    const: rockchip,rk3568-xpcs
+
+  reg:
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  clocks:
+    items:
+      - description: APB3 bus interface clock (clk_csr_i), required for register access
+      - description: EEE clock (clk_eee_i), required for Energy Efficient Ethernet operation
+
+  clock-names:
+    items:
+      - const: csr
+      - const: eee
+
+  phys:
+    maxItems: 1
+
+  phy-names:
+    const: serdes
+
+  power-domains:
+    maxItems: 1
+
+patternProperties:
+  "^ethernet-pcs-mii@[0-3]$":
+    type: object
+    description:
+      One of the four MII ports of the XPCS. The port is linked to an
+      Ethernet MAC controller via the pcs-handle property in the MAC's
+      device tree node.
+
+    properties:
+      reg:
+        description: MII port number.
+        enum: [0, 1, 2, 3]
+
+    required:
+      - reg
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+  - clocks
+  - clock-names
+  - phys
+  - phy-names
+  - power-domains
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rk3568-cru.h>
+    #include <dt-bindings/power/rk3568-power.h>
+    #include <dt-bindings/phy/phy.h>
+
+    ethernet-pcs@fda00000 {
+      compatible = "rockchip,rk3568-xpcs";
+      reg = <0xfda00000 0x200000>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+      clocks = <&cru PCLK_XPCS>, <&cru CLK_XPCS_EEE>;
+      clock-names = "csr", "eee";
+      phys = <&combphy2 PHY_TYPE_SGMII>;
+      phy-names = "serdes";
+      power-domains = <&power RK3568_PD_PIPE>;
+
+      ethernet-pcs-mii@0 {
+        reg = <0>;
+      };
+    };
-- 
2.47.3

[PATCH net-next v10 2/6] net: pcs: xpcs: add ANRESTART support for SGMII link recovery

[Coia Prant]

On some hardware using the DesignWare XPCS IP (e.g., RK3568 MAC side
SGMII), the PCS does not automatically restart auto-negotiation when the
link goes down and comes back up. Without an explicit ANRESTART, the link
stays down forever.

Add BMCR_ANRESTART in two places:
1. In xpcs_config_aneg_c37_sgmii(), when starting AN, set ANRESTART
   alongside ANENABLE to initiate a fresh negotiation.
2. In xpcs_get_state_c37_sgmii(), when link is down and AN completion is
   detected, clear the interrupt and trigger ANRESTART to restart the
   negotiation process. Propagate the return value of the restart so
   errors are not silently ignored.

The restart cannot go through the .pcs_an_restart op: phylink only
calls it for 802.3z interfaces, and SGMII is not one. Changing hardware
state from pcs_get_state() is already done elsewhere in this driver
(xpcs_get_state_c73() calls xpcs_soft_reset() and xpcs_do_config()),
so the same pattern is used here.

The latch is cleared before issuing the restart, not after: clearing it
afterwards would discard a freshly latched ANCMPLT from the new
negotiation. If an MDIO access fails at this point, it indicates an
unrecoverable hardware condition until reset.

Also clear DW_VR_MII_AN_INTR_STS in xpcs_config_aneg_c37_sgmii() before
starting AN, matching what xpcs_config_aneg_c37_1000basex() already does.
On the non-inband path the function now returns the result of that write
instead of the DIG_CTRL1 modify.

Update the comment in xpcs_config_aneg_c37_sgmii() to note that although
the DesignWare databook says AN restart is not needed for MAC side SGMII,
some implementations (e.g. Rockchip RK3568) require it to recover the
link after a disconnect.

This is not a fix for an existing mainline platform: the affected
platform (RK3568 XPCS) is introduced later in the same series.

Tested-by: Jiawen Wu <jiawenwu@trustnetic.com>
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 drivers/net/pcs/pcs-xpcs.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index b415b93d77c15..6466e0ff2a98b 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -761,7 +761,9 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
 	 *    DW xPCS used with DW EQoS MAC is always MAC side SGMII.
 	 * 4) VR_MII_DIG_CTRL1 Bit(9) [MAC_AUTO_SW] = 1b (Automatic
 	 *    speed/duplex mode change by HW after SGMII AN complete)
-	 * 5) VR_MII_MMD_CTRL Bit(12) [AN_ENABLE] = 1b (Enable SGMII AN)
+	 * 5) VR_MII_AN_INTR_STS = 0x0 (Clear CL37 AN complete status)
+	 * 6) VR_MII_MMD_CTRL Bit(12) [AN_ENABLE] = 1b (Enable SGMII AN)
+	 *    VR_MII_MMD_CTRL Bit(9) [AN_RESTART] = 1b (Restart SGMII AN)
 	 *
 	 * Note that VR_MII_MMD_CTRL is MII_BMCR.
 	 *
@@ -769,7 +771,14 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
 	 *	 SR_MII_AN_ADV. MAC side SGMII receives AN Tx Config from
 	 *	 PHY about the link state change after C28 AN is completed
 	 *	 between PHY and Link Partner. There is also no need to
-	 *	 trigger AN restart for MAC-side SGMII.
+	 *	 trigger AN restart for MAC-side SGMII on most devices.
+	 *
+	 * Note: While the DesignWare databook states that AN restart is
+	 *	 not needed for MAC side SGMII, some implementations (e.g.
+	 *	 Rockchip RK3568) exhibit a timing quirk when integrated with
+	 *	 phylink and do not restart AN automatically when the link
+	 *	 comes back up. An explicit AN restart is required on those
+	 *	 parts to recover the link after a disconnect.
 	 */
 	mdio_ctrl = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_BMCR);
 	if (mdio_ctrl < 0)
@@ -816,9 +825,14 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
 	if (ret < 0)
 		return ret;
 
+	/* Clear CL37 AN complete status */
+	ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
+	if (ret < 0)
+		return ret;
+
 	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
 		ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MII_BMCR,
-				 mdio_ctrl | BMCR_ANENABLE);
+				 mdio_ctrl | BMCR_ANENABLE | BMCR_ANRESTART);
 
 	return ret;
 }
@@ -1093,9 +1107,18 @@ static int xpcs_get_state_c37_sgmii(struct dw_xpcs *xpcs,
 		return 0;
 	}
 
-	/* Clear AN complete status or interrupt */
-	if (state->an_complete)
-		xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
+	if (state->an_complete) {
+		/* Clear AN complete status or interrupt */
+		ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
+		if (ret < 0)
+			return ret;
+
+		/* Initiate the next round of AN */
+		ret = xpcs_modify(xpcs, MDIO_MMD_VEND2, MII_BMCR, BMCR_ANRESTART,
+				  BMCR_ANRESTART);
+		if (ret < 0)
+			return ret;
+	}
 
 	return 0;
 }
-- 
2.47.3

[PATCH net-next v10 3/6] net: pcs: xpcs: add Rockchip RK3568 platform glue driver

[Coia Prant]

The RK3568 SoC integrates a Synopsys DesignWare XPCS that is accessed
via APB3 memory-mapped registers.  This driver provides the glue logic
to make the XPCS accessible to the generic pcs-xpcs core.

The XPCS block contains four MII ports (0..3), each of which can be
routed to GMAC0 or GMAC1 via the pcs-handle property in the MAC node.
The hardware maps these ports to different MMDs:
  - port 0: MMD 7 (ROCKCHIP_MMD_MII)
  - port 1: MMD 2 (ROCKCHIP_MMD_MII1)
  - port 2: MMD 3 (ROCKCHIP_MMD_MII2)
  - port 3: MMD 4 (ROCKCHIP_MMD_MII3)

This driver creates a virtual MDIO bus that translates MDIO operations
to APB3 register accesses, with proper address remapping for each port.
The generic xpcs driver then creates a phylink_pcs instance on top of
this bus, allowing the MAC to use the PCS via the standard phylink API.

The generic XPCS platform glue (pcs-xpcs-plat.o) is split out of the
pcs_xpcs composite object into its own module, gated behind the new
PCS_XPCS_PLATFORM symbol.  The symbol defaults to PCS_XPCS, so existing
configurations keep the snps,dw-xpcs platform glue enabled without any
change.

PCS_XPCS_ROCKCHIP selects GENERIC_PHY and PM_GENERIC_DOMAINS.
ARCH_ROCKCHIP already selects PM, so the dependency of PM_GENERIC_DOMAINS
on PM is satisfied on the target platform.

The EEE multiplier is derived at runtime from the EEE clock rate instead
of being hardcoded, because the clock is muxed between gpll200 (200 MHz)
and cpll125 (125 MHz) and can be changed by the board or firmware.  A
64-bit intermediate avoids overflow on 32-bit builds, and the result is
clamped to the 4-bit DW_VR_MII_EEE_MULT_FACT_100NS field.

Power management
================
The XPCS, its SerDes PHY and the SATA/PCIe controllers share the PD_PIPE
power domain on RK3568.  genpd powers the domain down during system
suspend once every consumer is suspended, which also kills the SerDes
and the XPCS.

Boards that rely on MAC WoL need the PCS and SerDes alive to receive
magic packets, so PD_PIPE must stay powered across system suspend.  Keep
it on by marking the XPCS as part of the wakeup path via
device_set_wakeup_path().  genpd then leaves the domain powered, because
the Rockchip power domain driver sets GENPD_FLAG_ACTIVE_WAKEUP on
PD_PIPE, which makes genpd check the wakeup path of its consumers during
system suspend.

This is unconditional because the XPCS core currently has no platform
callback through which a consumer could convey the MAC WoL state, and
the state itself is dynamic: WoL is toggled at runtime via ethtool, so
a static DT property cannot express it either.  The affected hardware
is limited to RK3568 boards using SGMII, which are always-on routers
where system suspend is not a realistic use case; the extra power draw
is therefore acceptable.

Runtime PM is unaffected: the runtime callbacks only gate the CSR clock,
and dev_pm_genpd_rpm_always_on() already keeps PD_PIPE on at runtime.

Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 59, CRU_CLKSEL_CON29)
Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf (Page 2078)
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 drivers/net/pcs/Kconfig         |  25 ++
 drivers/net/pcs/Makefile        |   5 +-
 drivers/net/pcs/pcs-xpcs-rk.c   | 619 ++++++++++++++++++++++++++++++++
 include/linux/pcs/pcs-xpcs-rk.h |  11 +
 4 files changed, 658 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/pcs/pcs-xpcs-rk.c
 create mode 100644 include/linux/pcs/pcs-xpcs-rk.h

diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
index e417fd66f660a..5382afcf95748 100644
--- a/drivers/net/pcs/Kconfig
+++ b/drivers/net/pcs/Kconfig
@@ -12,6 +12,31 @@ config PCS_XPCS
 	  This module provides a driver and helper functions for Synopsys
 	  DesignWare XPCS controllers.
 
+if PCS_XPCS
+
+config PCS_XPCS_PLATFORM
+	tristate "Generic XPCS controller support"
+	default PCS_XPCS
+	help
+	  Generic DWXPCS driver for platforms that don't require any
+	  platform specific code to function or is using platform
+	  data for setup.
+
+	  If you have a controller with this interface, say Y or M here.
+
+config PCS_XPCS_ROCKCHIP
+	tristate "Rockchip XPCS controller support"
+	default ARCH_ROCKCHIP
+	depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
+	select GENERIC_PHY
+	select PM_GENERIC_DOMAINS if PM
+	help
+	  Support for XPCS controller on Rockchip RK356x SoC.
+
+	  If you have a Rockchip SoC with this interface, say Y or M here.
+
+endif # PCS_XPCS
+
 config PCS_LYNX
 	tristate
 	help
diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
index 4f7920618b900..f9f6cf2578d72 100644
--- a/drivers/net/pcs/Makefile
+++ b/drivers/net/pcs/Makefile
@@ -1,10 +1,11 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for Linux PCS drivers
 
-pcs_xpcs-$(CONFIG_PCS_XPCS)	:= pcs-xpcs.o pcs-xpcs-plat.o \
-				   pcs-xpcs-nxp.o pcs-xpcs-wx.o
+pcs_xpcs-$(CONFIG_PCS_XPCS)	:= pcs-xpcs.o pcs-xpcs-nxp.o pcs-xpcs-wx.o
 
 obj-$(CONFIG_PCS_XPCS)		+= pcs_xpcs.o
+obj-$(CONFIG_PCS_XPCS_PLATFORM) += pcs-xpcs-plat.o
+obj-$(CONFIG_PCS_XPCS_ROCKCHIP) += pcs-xpcs-rk.o
 obj-$(CONFIG_PCS_LYNX)		+= pcs-lynx.o
 obj-$(CONFIG_PCS_MTK_LYNXI)	+= pcs-mtk-lynxi.o
 obj-$(CONFIG_PCS_RZN1_MIIC)	+= pcs-rzn1-miic.o
diff --git a/drivers/net/pcs/pcs-xpcs-rk.c b/drivers/net/pcs/pcs-xpcs-rk.c
new file mode 100644
index 0000000000000..35ee980a759e5
--- /dev/null
+++ b/drivers/net/pcs/pcs-xpcs-rk.c
@@ -0,0 +1,619 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Rockchip XPCS platform device driver
+ *
+ * Based on the Synopsys DesignWare XPCS platform driver.
+ * Copyright (C) 2024 Serge Semin
+ *
+ * Adapted for Rockchip SoCs, with reference to the Rockchip OEM driver.
+ * Copyright (C) 2026 Coia Prant
+ */
+
+#include <linux/atomic.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/math.h>
+#include <linux/mdio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pcs/pcs-xpcs-rk.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/property.h>
+#include <linux/sizes.h>
+#include <linux/time.h>
+
+#include "pcs-xpcs.h"
+
+struct dw_xpcs_rk {
+	struct platform_device *pdev;
+	struct mii_bus *bus;
+	void __iomem *reg_base;
+	struct phy *serdes_phy;
+	struct clk *csr_clk;
+	struct clk *eee_clk;
+	u8 eee_mult_fact;
+};
+
+static ptrdiff_t xpcs_rk_addr_format(int dev, int reg)
+{
+	return FIELD_PREP(0x70000, dev) | FIELD_PREP(0xffff, reg);
+}
+
+static int xpcs_rk_read_reg(struct dw_xpcs_rk *pxpcs, int dev, int reg)
+{
+	ptrdiff_t csr;
+	int ret;
+
+	csr = xpcs_rk_addr_format(dev, reg);
+
+	ret = pm_runtime_resume_and_get(&pxpcs->pdev->dev);
+	if (ret)
+		return ret;
+
+	ret = readl(pxpcs->reg_base + (csr << 2)) & 0xffff;
+
+	pm_runtime_put(&pxpcs->pdev->dev);
+	return ret;
+}
+
+static int xpcs_rk_write_reg(struct dw_xpcs_rk *pxpcs, int dev, int reg, u16 val)
+{
+	ptrdiff_t csr;
+	int ret;
+
+	csr = xpcs_rk_addr_format(dev, reg);
+
+	ret = pm_runtime_resume_and_get(&pxpcs->pdev->dev);
+	if (ret)
+		return ret;
+
+	writel(val, pxpcs->reg_base + (csr << 2));
+
+	pm_runtime_put(&pxpcs->pdev->dev);
+	return 0;
+}
+
+#define ROCKCHIP_MMD_MII1	2
+#define ROCKCHIP_MMD_MII2	3
+#define ROCKCHIP_MMD_MII3	4
+#define ROCKCHIP_MMD_PMAPMD	6
+#define ROCKCHIP_MMD_MII	7
+
+static bool xpcs_rk_mdio_addr_validate(int addr)
+{
+	return !(addr < 0 || addr > 3);
+}
+
+static int xpcs_rk_mdio_read_remapping(int addr, int dev, int reg)
+{
+	switch (dev) {
+	case MDIO_MMD_PMAPMD:
+		return ROCKCHIP_MMD_PMAPMD;
+	case MDIO_MMD_VEND2:
+		break;
+	default:
+		return -ENXIO;
+	}
+
+	/*
+	 * Reads are redirected by hardware to the port's read-only mirror;
+	 * only writes have to be targeted at MII (see the write path).
+	 */
+	switch (addr) {
+	case 0:
+		return ROCKCHIP_MMD_MII;
+	case 1:
+		return ROCKCHIP_MMD_MII1;
+	case 2:
+		return ROCKCHIP_MMD_MII2;
+	case 3:
+		return ROCKCHIP_MMD_MII3;
+	default:
+		return -ENODEV;
+	}
+}
+
+static int xpcs_rk_mdio_write_remapping(int addr, int dev, int reg)
+{
+	switch (dev) {
+	case MDIO_MMD_PMAPMD:
+		return ROCKCHIP_MMD_PMAPMD;
+	case MDIO_MMD_VEND2:
+		break;
+	default:
+		return -ENXIO;
+	}
+
+	/*
+	 * These registers physically live only in MII (the management port).
+	 * Ports 1-3 expose read-only mirrors of these bits, so writes must
+	 * always target MII; the read path remaps per address and the
+	 * hardware redirects to the port's mirror.
+	 */
+	switch (reg) {
+	case DW_VR_MII_AN_CTRL:
+	case DW_VR_MII_AN_INTR_STS:
+	case DW_VR_MII_EEE_MCTRL0:
+	case DW_VR_MII_EEE_MCTRL1:
+	case DW_VR_MII_DIG_CTRL2:
+		return ROCKCHIP_MMD_MII;
+	default:
+		break;
+	}
+
+	switch (addr) {
+	case 0:
+		return ROCKCHIP_MMD_MII;
+	case 1:
+		return ROCKCHIP_MMD_MII1;
+	case 2:
+		return ROCKCHIP_MMD_MII2;
+	case 3:
+		return ROCKCHIP_MMD_MII3;
+	default:
+		return -ENODEV;
+	}
+}
+
+static int xpcs_rk_read_c22(struct mii_bus *bus, int addr, int reg)
+{
+	struct dw_xpcs_rk *pxpcs = bus->priv;
+	int dev;
+
+	if (!xpcs_rk_mdio_addr_validate(addr))
+		return -ENODEV;
+
+	dev = xpcs_rk_mdio_read_remapping(addr, MDIO_MMD_VEND2, reg);
+	if (dev < 0)
+		return 0xffff;
+
+	return xpcs_rk_read_reg(pxpcs, dev, reg);
+}
+
+static int xpcs_rk_write_c22(struct mii_bus *bus, int addr, int reg, u16 val)
+{
+	struct dw_xpcs_rk *pxpcs = bus->priv;
+	int dev;
+
+	if (!xpcs_rk_mdio_addr_validate(addr))
+		return -ENODEV;
+
+	dev = xpcs_rk_mdio_write_remapping(addr, MDIO_MMD_VEND2, reg);
+	if (dev < 0)
+		return 0;
+
+	return xpcs_rk_write_reg(pxpcs, dev, reg, val);
+}
+
+static int xpcs_rk_read_c45(struct mii_bus *bus, int addr, int dev, int reg)
+{
+	struct dw_xpcs_rk *pxpcs = bus->priv;
+
+	if (!xpcs_rk_mdio_addr_validate(addr))
+		return -ENODEV;
+
+	dev = xpcs_rk_mdio_read_remapping(addr, dev, reg);
+	if (dev < 0)
+		return 0xffff;
+
+	return xpcs_rk_read_reg(pxpcs, dev, reg);
+}
+
+static int xpcs_rk_write_c45(struct mii_bus *bus, int addr, int dev, int reg, u16 val)
+{
+	struct dw_xpcs_rk *pxpcs = bus->priv;
+
+	if (!xpcs_rk_mdio_addr_validate(addr))
+		return -ENODEV;
+
+	dev = xpcs_rk_mdio_write_remapping(addr, dev, reg);
+	if (dev < 0)
+		return 0;
+
+	return xpcs_rk_write_reg(pxpcs, dev, reg, val);
+}
+
+static struct dw_xpcs_rk *xpcs_rk_create_data(struct platform_device *pdev)
+{
+	struct dw_xpcs_rk *pxpcs;
+
+	pxpcs = devm_kzalloc(&pdev->dev, sizeof(*pxpcs), GFP_KERNEL);
+	if (!pxpcs)
+		return ERR_PTR(-ENOMEM);
+
+	pxpcs->pdev = pdev;
+
+	dev_set_drvdata(&pdev->dev, pxpcs);
+
+	return pxpcs;
+}
+
+static int xpcs_rk_serdes_phy_init(struct dw_xpcs_rk *pxpcs)
+{
+	struct device *dev = &pxpcs->pdev->dev;
+
+	pxpcs->serdes_phy = devm_phy_get(dev, "serdes");
+	if (IS_ERR(pxpcs->serdes_phy))
+		return dev_err_probe(dev, PTR_ERR(pxpcs->serdes_phy),
+					"Failed to get SerDes PHY\n");
+
+	return 0;
+}
+
+static void xpcs_rk_serdes_phy_poweroff(void *data)
+{
+	struct dw_xpcs_rk *pxpcs = data;
+	struct device *dev = &pxpcs->pdev->dev;
+
+	phy_power_off(pxpcs->serdes_phy);
+	phy_exit(pxpcs->serdes_phy);
+
+	dev_pm_genpd_rpm_always_on(dev, false);
+}
+
+static int xpcs_rk_serdes_phy_poweron(struct dw_xpcs_rk *pxpcs)
+{
+	struct device *dev = &pxpcs->pdev->dev;
+	int ret;
+
+	/*
+	 * The power domain is required and must be enabled, which allows us to
+	 * dynamically turn the CSR clock on/off using PM while keeping the PCS
+	 * powered on.
+	 */
+	ret = dev_pm_genpd_rpm_always_on(dev, true);
+	if (ret) {
+		dev_err(dev, "Failed to power on power-domains\n");
+		return ret;
+	}
+
+	ret = phy_init(pxpcs->serdes_phy);
+	if (ret) {
+		dev_err(dev, "Failed to init SerDes PHY\n");
+		goto pm_domain;
+	}
+
+	ret = phy_power_on(pxpcs->serdes_phy);
+	if (ret) {
+		dev_err(dev, "Failed to power on SerDes PHY\n");
+		goto serdes_phy;
+	}
+
+	ret = devm_add_action_or_reset(dev, xpcs_rk_serdes_phy_poweroff, pxpcs);
+	if (ret) {
+		dev_err(dev, "Failed to register devm for SerDes PHY: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+
+serdes_phy:
+	phy_exit(pxpcs->serdes_phy);
+pm_domain:
+	dev_pm_genpd_rpm_always_on(dev, false);
+	return ret;
+}
+
+static int xpcs_rk_init_res(struct dw_xpcs_rk *pxpcs)
+{
+	struct platform_device *pdev = pxpcs->pdev;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "No reg-space found\n");
+		return -EINVAL;
+	}
+
+	if (resource_size(res) < SZ_2M) {
+		dev_err(dev, "Invalid reg-space size\n");
+		return -EINVAL;
+	}
+
+	pxpcs->reg_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(pxpcs->reg_base)) {
+		dev_err(dev, "Failed to map reg-space\n");
+		return PTR_ERR(pxpcs->reg_base);
+	}
+
+	return 0;
+}
+
+static void xpcs_rk_exit_clk(void *data)
+{
+	struct dw_xpcs_rk *pxpcs = data;
+	struct device *dev = &pxpcs->pdev->dev;
+
+	pm_runtime_force_suspend(dev);
+	clk_disable_unprepare(pxpcs->eee_clk);
+}
+
+static int xpcs_rk_init_clk(struct dw_xpcs_rk *pxpcs)
+{
+	struct device *dev = &pxpcs->pdev->dev;
+	unsigned long rate;
+	u64 mult;
+	int ret;
+
+	pxpcs->csr_clk = devm_clk_get(dev, "csr");
+	if (IS_ERR(pxpcs->csr_clk))
+		return dev_err_probe(dev, PTR_ERR(pxpcs->csr_clk),
+					 "Failed to get CSR clock\n");
+
+	pxpcs->eee_clk = devm_clk_get(dev, "eee");
+	if (IS_ERR(pxpcs->eee_clk))
+		return dev_err_probe(dev, PTR_ERR(pxpcs->eee_clk),
+					 "Failed to get EEE clock\n");
+
+	ret = clk_prepare_enable(pxpcs->eee_clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable EEE clock\n");
+		return ret;
+	}
+
+	pm_runtime_set_suspended(dev);
+	pm_runtime_enable(dev);
+
+	ret = devm_add_action_or_reset(dev, xpcs_rk_exit_clk, pxpcs);
+	if (ret) {
+		dev_err(dev, "Failed to register devm for EEE clock: %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * Compute the multiplier for the EEE clock so that
+	 * clk_eee_period * (mult_fact + 1) falls within 80..120 ns.
+	 *
+	 * On RK3568, clk_xpcs_eee is muxed between gpll200 (200 MHz, 5 ns)
+	 * and cpll125 (125 MHz, 8 ns), selected by CRU_CLKSEL_CON29 bit 13.
+	 * The reset value is 0 (200 MHz), but derive the value at runtime to
+	 * stay correct if the mux is changed by a board.
+	 *
+	 * Use a 64-bit intermediate: on 32-bit builds, 100 * 200000000
+	 * does not fit in unsigned long. Clamp to the 4-bit
+	 * DW_VR_MII_EEE_MULT_FACT_100NS field. The mux only provides
+	 * 125 MHz or 200 MHz, so the rate cannot drop below the 5 MHz
+	 * threshold where DIV_ROUND_CLOSEST_ULL() would return 0 and the
+	 * subtraction below would underflow.
+	 */
+	rate = clk_get_rate(pxpcs->eee_clk);
+	if (!rate)
+		return dev_err_probe(dev, -EINVAL, "Invalid EEE clock rate\n");
+
+	mult = DIV_ROUND_CLOSEST_ULL(100ULL * rate, NSEC_PER_SEC) - 1;
+	pxpcs->eee_mult_fact = min_t(u64, mult, 15);
+	return 0;
+}
+
+static int xpcs_rk_init_bus(struct dw_xpcs_rk *pxpcs)
+{
+	struct device *dev = &pxpcs->pdev->dev;
+	static atomic_t id = ATOMIC_INIT(-1);
+	struct mii_bus *bus;
+	int ret;
+
+	bus = devm_mdiobus_alloc_size(dev, 0);
+	if (!bus)
+		return -ENOMEM;
+
+	bus->name = "Rockchip DW XPCS MCI/APB3";
+	bus->read = xpcs_rk_read_c22;
+	bus->write = xpcs_rk_write_c22;
+	bus->read_c45 = xpcs_rk_read_c45;
+	bus->write_c45 = xpcs_rk_write_c45;
+	bus->phy_mask = ~0;
+	bus->parent = dev;
+	bus->priv = pxpcs;
+
+	snprintf(bus->id, MII_BUS_ID_SIZE,
+		 "rockchip_dwxpcs-%x", atomic_inc_return(&id));
+
+	/*
+	 * MDIO-bus here serves as just a back-end engine abstracting out
+	 * the MDIO and MCI/APB3 IO interfaces utilized for the Rockchip DWXPCS CSRs
+	 * access.
+	 */
+	ret = devm_mdiobus_register(dev, bus);
+	if (ret) {
+		dev_err(dev, "Failed to create MDIO bus\n");
+		return ret;
+	}
+
+	pxpcs->bus = bus;
+	return 0;
+}
+
+static int xpcs_rk_probe(struct platform_device *pdev)
+{
+	struct dw_xpcs_rk *pxpcs;
+	int ret;
+
+	pxpcs = xpcs_rk_create_data(pdev);
+	if (IS_ERR(pxpcs))
+		return PTR_ERR(pxpcs);
+
+	/*
+	 * The XPCS lives in the PD_PIPE power domain. The domain must be
+	 * powered on before any register access, otherwise the SoC will
+	 * trigger a synchronous external abort (SError).
+	 *
+	 * Accessing the XPCS registers also requires a TX clock from the
+	 * SerDes, which is needed for the soft reset.
+	 */
+	ret = xpcs_rk_serdes_phy_init(pxpcs);
+	if (ret)
+		return ret;
+
+	ret = xpcs_rk_serdes_phy_poweron(pxpcs);
+	if (ret)
+		return ret;
+
+	ret = xpcs_rk_init_res(pxpcs);
+	if (ret)
+		return ret;
+
+	ret = xpcs_rk_init_clk(pxpcs);
+	if (ret)
+		return ret;
+
+	ret = xpcs_rk_init_bus(pxpcs);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct of_device_id xpcs_rk_of_ids[] = {
+	{ .compatible = "rockchip,rk3568-xpcs" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, xpcs_rk_of_ids);
+
+struct dw_xpcs *xpcs_rk_create(struct device *dev, struct device_node *np)
+{
+	struct platform_device *pdev;
+	struct device_node *pcs_np;
+	struct device_link *link;
+	struct dw_xpcs_rk *pxpcs;
+	struct dw_xpcs *xpcs;
+	u32 port;
+
+	if (!of_device_is_available(np))
+		return ERR_PTR(-ENODEV);
+
+	if (of_property_read_u32(np, "reg", &port))
+		return ERR_PTR(-EINVAL);
+
+	if (!xpcs_rk_mdio_addr_validate((int)port))
+		return ERR_PTR(-EINVAL);
+
+	/* The XPCS pdev is attached to the parent node */
+	pcs_np = of_get_parent(np);
+	if (!pcs_np)
+		return ERR_PTR(-ENODEV);
+
+	if (!of_device_is_available(pcs_np)) {
+		of_node_put(pcs_np);
+		return ERR_PTR(-ENODEV);
+	}
+
+	if (!of_match_node(xpcs_rk_of_ids, pcs_np)) {
+		of_node_put(pcs_np);
+		return ERR_PTR(-EINVAL);
+	}
+
+	pdev = of_find_device_by_node(pcs_np);
+	of_node_put(pcs_np);
+	if (!pdev)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	/*
+	 * Establish the device link before reading the supplier's drvdata.
+	 * device_link_add() does not fail on a supplier that is unbinding:
+	 * it creates the link in DL_STATE_SUPPLIER_UNBIND. Whether the link
+	 * actually protects the drvdata depends on the supplier's state at
+	 * creation time.
+	 *
+	 * Check link->supplier->links.status right after creation. If the
+	 * supplier was DL_DEV_DRIVER_BOUND, the link is in
+	 * DL_STATE_CONSUMER_PROBE and device_links_unbind_consumers() will
+	 * wait for this probe to finish before unbinding the supplier, so
+	 * the drvdata stays valid for the rest of the function. Any other
+	 * state means the supplier is not usable yet; defer and retry.
+	 *
+	 * The link is released automatically when the consumer device is
+	 * destroyed (DL_FLAG_AUTOREMOVE_CONSUMER), so no explicit
+	 * device_link_remove() is needed on the failure paths.
+	 */
+	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
+	if (!link) {
+		put_device(&pdev->dev);
+		return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	if (READ_ONCE(link->supplier->links.status) != DL_DEV_DRIVER_BOUND) {
+		put_device(&pdev->dev);
+		return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	pxpcs = platform_get_drvdata(pdev);
+	if (!pxpcs || !pxpcs->bus) {
+		put_device(&pdev->dev);
+		return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	xpcs = xpcs_create_mdiodev(pxpcs->bus, (int)port);
+	if (IS_ERR(xpcs)) {
+		put_device(&pdev->dev);
+		return xpcs;
+	}
+
+	xpcs_config_eee_mult_fact(xpcs, pxpcs->eee_mult_fact);
+	put_device(&pdev->dev);
+	return xpcs;
+}
+EXPORT_SYMBOL_GPL(xpcs_rk_create);
+
+static int xpcs_rk_pm_runtime_suspend(struct device *dev)
+{
+	struct dw_xpcs_rk *pxpcs = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(pxpcs->csr_clk);
+
+	return 0;
+}
+
+static int xpcs_rk_pm_runtime_resume(struct device *dev)
+{
+	struct dw_xpcs_rk *pxpcs = dev_get_drvdata(dev);
+
+	return clk_prepare_enable(pxpcs->csr_clk);
+}
+
+static int xpcs_rk_system_suspend(struct device *dev)
+{
+	/*
+	 * Keep the PD_PIPE power domain on during system suspend.
+	 *
+	 * PD_PIPE is shared with SATA/PCIe and would be powered down by
+	 * genpd once all its consumers are suspended, killing the SerDes
+	 * and breaking MAC WoL.  Mark the XPCS as part of the wakeup path
+	 * so genpd keeps the domain on.  Unconditional because the XPCS
+	 * core has no callback to convey the MAC WoL state.
+	 */
+	device_set_wakeup_path(dev);
+	return 0;
+}
+
+static int xpcs_rk_system_resume(struct device *dev)
+{
+	return 0;
+}
+
+static _DEFINE_DEV_PM_OPS(xpcs_rk_pm_ops,
+			  xpcs_rk_system_suspend, xpcs_rk_system_resume,
+			  xpcs_rk_pm_runtime_suspend, xpcs_rk_pm_runtime_resume,
+			  NULL);
+
+static struct platform_driver xpcs_rk_driver = {
+	.probe = xpcs_rk_probe,
+	.driver = {
+		.name = "rk_xpcs-dwxpcs",
+		.pm = pm_ptr(&xpcs_rk_pm_ops),
+		.of_match_table = xpcs_rk_of_ids,
+	},
+};
+module_platform_driver(xpcs_rk_driver);
+
+MODULE_DESCRIPTION("Rockchip XPCS platform device driver");
+MODULE_AUTHOR("Coia Prant <coiaprant@gmail.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/pcs/pcs-xpcs-rk.h b/include/linux/pcs/pcs-xpcs-rk.h
new file mode 100644
index 0000000000000..28723d5bd75cc
--- /dev/null
+++ b/include/linux/pcs/pcs-xpcs-rk.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_PCS_XPCS_ROCKCHIP_H
+#define __LINUX_PCS_XPCS_ROCKCHIP_H
+
+#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/pcs/pcs-xpcs.h>
+
+struct dw_xpcs *xpcs_rk_create(struct device *dev, struct device_node *np);
+
+#endif /* __LINUX_PCS_XPCS_ROCKCHIP_H */
-- 
2.47.3

[PATCH net-next v10 4/6] dt-bindings: net: rockchip-dwmac: document pcs-handle

[Coia Prant]

The Rockchip GMAC binding needs to describe the PCS reference used by
the SGMII support added later in this series. The property will be
parsed by rk_pcs_init(), and a missing phandle fails the probe. Add it
and require it when phy-mode is "sgmii" on rockchip,rk3568-gmac, the
only SoC in this binding that has SGMII support.

Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 .../bindings/net/rockchip-dwmac.yaml           | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
index 80c252845349c..bb7540e838033 100644
--- a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
@@ -120,6 +120,12 @@ properties:
     maximum: 0x7F
     default: 0x10
 
+  pcs-handle:
+    description:
+      Specifies a reference to a node representing the PCS device
+      connected to this GMAC. Required when phy-mode is "sgmii".
+    maxItems: 1
+
   phy-supply:
     description: PHY regulator
 
@@ -159,6 +165,18 @@ allOf:
         clocks:
           minItems: 5
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: rockchip,rk3568-gmac
+        phy-mode:
+          contains:
+            const: sgmii
+    then:
+      required:
+        - pcs-handle
+
 unevaluatedProperties: false
 
 examples:
-- 
2.47.3

[PATCH net-next v10 5/6] net: stmmac: dwmac-rk: add SGMII support for RK3568

[Coia Prant]

The RK3568 SoC integrates a Synopsys DesignWare XPCS that can be
connected to GMAC0 or GMAC1 in SGMII mode.  Add the necessary glue
logic to support this configuration.

The current dwmac-rk driver does not support SGMII mode.  SGMII
requires a PCS to handle auto-negotiation and link state reporting,
but the existing driver only supports RGMII and RMII.

Add a set_to_sgmii() callback to configure the GMAC GRF register for
SGMII mode (bit 7 set, interface selection bits 4:6 ignored when set).
Also add a set_to_rmii() callback for rk3568 to explicitly clear bit 7,
since the new SGMII path leaves it set and the RMII branch previously
relied on the SoC reset value.

Provide pcs_init/pcs_exit callbacks to create/destroy the XPCS via
xpcs_rk_create() from the Rockchip XPCS platform driver, and a
select_pcs callback to return the XPCS to phylink. SGMII is not added
to rk_get_interfaces(): it comes from the XPCS's own
supported_interfaces, merged by stmmac_phylink_setup().

The SerDes PHY and the PD_PIPE power domain are owned by the XPCS
driver rather than managed through the stmmac
serdes_poweron/serdes_poweroff callbacks, which are legacy and meant
for single-MAC platforms. On RK3568 the XPCS is the natural owner of
the shared SerDes.

DWMAC_ROCKCHIP selects PCS_XPCS_ROCKCHIP. PCS_XPCS itself is already
selected by STMMAC_ETH, and PM is selected by ARCH_ROCKCHIP, so no
further selects are needed.

Reorder rk_gmac_powerup() so that gmac_clk_enable() is called before
the SGMII check.  The SGMII path skips rk_get_phy_intf_sel(), so the
clock must be enabled earlier to cover all register accesses in that
path.  While at it, unify the error unwinding into a single
clk_disable label and add error handling for the default (unhandled
interface) case.

SGMII In-band vs Out-of-band
============================
On RK3568, the MAC clock is fixed at 125 MHz and cannot be dynamically
changed by the stmmac core's set_clk_tx_rate callback.  In-band mode
works because the PCS handles rate adaptation internally.  Out-of-band
mode does not work because the MAC would need to change the clock rate
to 125/12.5/1.25 MHz for 1000/100/10 Mbps respectively, and the clock
is fixed.

Enable default_an_inband for SGMII and disable the generic stmmac
set_clk_tx_rate callback.  This forces phylink to use in-band mode,
where the PCS is responsible for speed/duplex negotiation.

Note that default_an_inband can be overridden by a fixed-link node,
and phylink may also fall back to out-of-band if the PHY does not
support in-band signalling.  Out-of-band SGMII is not supported by
this driver: the MAC clock would stay at 125 MHz for 10/100 Mbps,
giving working TX but failing RX.  Boards must use in-band mode
(managed = "in-band-status" or an in-band capable PHY).

Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 386)
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |   1 +
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 130 +++++++++++++++---
 2 files changed, 111 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index e3dd5adda5aca..5088acc06982e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -170,6 +170,7 @@ config DWMAC_ROCKCHIP
 	default ARCH_ROCKCHIP
 	depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
 	select MFD_SYSCON
+	select PCS_XPCS_ROCKCHIP
 	help
 	  Support for Ethernet controller on Rockchip RK3288 SoC.
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 72bdbcb5e863a..3b50ad3c600b9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -20,6 +20,7 @@
 #include <linux/delay.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/pcs/pcs-xpcs-rk.h>
 #include <linux/pm_runtime.h>
 
 #include "stmmac_platform.h"
@@ -47,6 +48,7 @@ struct rk_gmac_ops {
 	void (*set_to_rgmii)(struct rk_priv_data *bsp_priv,
 			     int tx_delay, int rx_delay);
 	void (*set_to_rmii)(struct rk_priv_data *bsp_priv);
+	void (*set_to_sgmii)(struct rk_priv_data *bsp_priv);
 	int (*set_speed)(struct rk_priv_data *bsp_priv,
 			 phy_interface_t interface, int speed);
 	void (*integrated_phy_powerup)(struct rk_priv_data *bsp_priv);
@@ -63,6 +65,7 @@ struct rk_gmac_ops {
 	bool clock_grf_reg_in_php;
 	bool supports_rgmii;
 	bool supports_rmii;
+	bool supports_sgmii;
 	bool php_grf_required;
 	bool regs_valid;
 	u32 regs[];
@@ -98,6 +101,7 @@ struct rk_priv_data {
 	bool integrated_phy;
 	bool supports_rgmii;
 	bool supports_rmii;
+	bool supports_sgmii;
 
 	struct clk_bulk_data *clks;
 	int num_clks;
@@ -809,6 +813,8 @@ static const struct rk_gmac_ops rk3528_ops = {
 #define RK3568_GRF_GMAC1_CON1		0x038c
 
 /* RK3568_GRF_GMAC0_CON1 && RK3568_GRF_GMAC1_CON1 */
+#define RK3568_GMAC_MODE_RMII_RGMII		GRF_CLR_BIT(7)
+#define RK3568_GMAC_MODE_SGMII_QSGMII		GRF_BIT(7)
 #define RK3568_GMAC_FLOW_CTRL			GRF_BIT(3)
 #define RK3568_GMAC_FLOW_CTRL_CLR		GRF_CLR_BIT(3)
 #define RK3568_GMAC_RXCLK_DLY_ENABLE		GRF_BIT(1)
@@ -836,6 +842,16 @@ static int rk3568_init(struct rk_priv_data *bsp_priv)
 	}
 }
 
+static void rk3568_set_to_rmii(struct rk_priv_data *bsp_priv)
+{
+	u32 con1;
+
+	con1 = (bsp_priv->id == 1) ? RK3568_GRF_GMAC1_CON1 :
+				     RK3568_GRF_GMAC0_CON1;
+
+	regmap_write(bsp_priv->grf, con1, RK3568_GMAC_MODE_RMII_RGMII);
+}
+
 static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
 				int tx_delay, int rx_delay)
 {
@@ -851,19 +867,31 @@ static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
 
 	regmap_write(bsp_priv->grf, con1,
+		     RK3568_GMAC_MODE_RMII_RGMII |
 		     RK3568_GMAC_RXCLK_DLY_ENABLE |
 		     RK3568_GMAC_TXCLK_DLY_ENABLE);
 }
 
+static void rk3568_set_to_sgmii(struct rk_priv_data *bsp_priv)
+{
+	u32 con1;
+
+	con1 = (bsp_priv->id == 1) ? RK3568_GRF_GMAC1_CON1 :
+				     RK3568_GRF_GMAC0_CON1;
+
+	regmap_write(bsp_priv->grf, con1, RK3568_GMAC_MODE_SGMII_QSGMII);
+}
+
 static const struct rk_gmac_ops rk3568_ops = {
 	.init = rk3568_init,
+	.set_to_rmii = rk3568_set_to_rmii,
 	.set_to_rgmii = rk3568_set_to_rgmii,
+	.set_to_sgmii = rk3568_set_to_sgmii,
+
 	.set_speed = rk_set_clk_mac_speed,
 
 	.gmac_phy_intf_sel_mask = GENMASK_U16(6, 4),
 
-	.supports_rmii = true,
-
 	.regs_valid = true,
 	.regs = {
 		0xfe2a0000, /* gmac0 */
@@ -1211,6 +1239,43 @@ static void rk_phy_powerdown(struct rk_priv_data *bsp_priv)
 		dev_err(bsp_priv->dev, "fail to disable phy-supply\n");
 }
 
+static int rk_pcs_init(struct stmmac_priv *priv)
+{
+	struct device_node *np = priv->device->of_node;
+	struct device_node *pcs_node;
+	struct dw_xpcs *xpcs;
+
+	pcs_node = of_parse_phandle(np, "pcs-handle", 0);
+	if (!pcs_node)
+		return -ENODEV;
+
+	xpcs = xpcs_rk_create(priv->device, pcs_node);
+	of_node_put(pcs_node);
+	if (IS_ERR(xpcs))
+		return PTR_ERR(xpcs);
+
+	priv->hw->xpcs = xpcs;
+	return 0;
+}
+
+static void rk_pcs_exit(struct stmmac_priv *priv)
+{
+	if (!priv->hw->xpcs)
+		return;
+
+	xpcs_destroy(priv->hw->xpcs);
+	priv->hw->xpcs = NULL;
+}
+
+static struct phylink_pcs *rk_select_pcs(struct stmmac_priv *priv,
+					 phy_interface_t interface)
+{
+	if (!priv->hw->xpcs)
+		return NULL;
+
+	return xpcs_to_phylink_pcs(priv->hw->xpcs);
+}
+
 static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
 					  struct plat_stmmacenet_data *plat,
 					  const struct rk_gmac_ops *ops)
@@ -1333,6 +1398,7 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
 
 	bsp_priv->supports_rgmii = ops->supports_rgmii || !!ops->set_to_rgmii;
 	bsp_priv->supports_rmii = ops->supports_rmii || !!ops->set_to_rmii;
+	bsp_priv->supports_sgmii = ops->supports_sgmii || !!ops->set_to_sgmii;
 
 	if (ops->init) {
 		ret = ops->init(bsp_priv);
@@ -1364,6 +1430,10 @@ static int rk_gmac_check_ops(struct rk_priv_data *bsp_priv)
 		if (!bsp_priv->supports_rmii)
 			return -EINVAL;
 		break;
+	case PHY_INTERFACE_MODE_SGMII:
+		if (!bsp_priv->supports_sgmii)
+			return -EINVAL;
+		break;
 	default:
 		dev_err(bsp_priv->dev,
 			"unsupported interface %d", bsp_priv->phy_iface);
@@ -1382,16 +1452,19 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
 	if (ret)
 		return ret;
 
+	ret = gmac_clk_enable(bsp_priv, true);
+	if (ret)
+		return ret;
+
+	if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_SGMII)
+		goto set_mode;
+
 	ret = rk_get_phy_intf_sel(bsp_priv->phy_iface);
 	if (ret < 0)
-		return ret;
+		goto clk_disable;
 
 	intf = ret;
 
-	ret = gmac_clk_enable(bsp_priv, true);
-	if (ret)
-		return ret;
-
 	if (bsp_priv->gmac_phy_intf_sel_mask ||
 	    bsp_priv->gmac_rmii_mode_mask) {
 		/* If defined, encode the phy_intf_sel value */
@@ -1402,10 +1475,8 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
 				      bsp_priv->gmac_rmii_mode_mask);
 
 		ret = rk_write_gmac_grf_reg(bsp_priv, val);
-		if (ret < 0) {
-			gmac_clk_enable(bsp_priv, false);
-			return ret;
-		}
+		if (ret < 0)
+			goto clk_disable;
 	}
 
 	if (bsp_priv->clock.rmii_mode_mask) {
@@ -1413,13 +1484,12 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
 				     bsp_priv->clock.rmii_mode_mask);
 
 		ret = rk_write_clock_grf_reg(bsp_priv, val);
-		if (ret < 0) {
-			gmac_clk_enable(bsp_priv, false);
-			return ret;
-		}
+		if (ret < 0)
+			goto clk_disable;
 	}
 
-	/*rmii or rgmii*/
+set_mode:
+	/* rmii, rgmii, sgmii */
 	switch (bsp_priv->phy_iface) {
 	case PHY_INTERFACE_MODE_RGMII:
 		dev_info(dev, "init for RGMII\n");
@@ -1450,15 +1520,20 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
 		if (bsp_priv->ops->set_to_rmii)
 			bsp_priv->ops->set_to_rmii(bsp_priv);
 		break;
+	case PHY_INTERFACE_MODE_SGMII:
+		dev_info(dev, "init for SGMII\n");
+		if (bsp_priv->ops->set_to_sgmii)
+			bsp_priv->ops->set_to_sgmii(bsp_priv);
+		break;
 	default:
 		dev_err(dev, "NO interface defined!\n");
+		ret = -EINVAL;
+		goto clk_disable;
 	}
 
 	ret = rk_phy_powerup(bsp_priv);
-	if (ret) {
-		gmac_clk_enable(bsp_priv, false);
-		return ret;
-	}
+	if (ret)
+		goto clk_disable;
 
 	pm_runtime_get_sync(dev);
 
@@ -1466,6 +1541,10 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
 		bsp_priv->ops->integrated_phy_powerup(bsp_priv);
 
 	return 0;
+
+clk_disable:
+	gmac_clk_enable(bsp_priv, false);
+	return ret;
 }
 
 static void rk_gmac_powerdown(struct rk_priv_data *gmac)
@@ -1605,6 +1684,17 @@ static int rk_gmac_probe(struct platform_device *pdev)
 	plat_dat->suspend = rk_gmac_suspend;
 	plat_dat->resume = rk_gmac_resume;
 
+	if (plat_dat->phy_interface == PHY_INTERFACE_MODE_SGMII) {
+		/* SGMII clock always runs at 125 MHz */
+		plat_dat->set_clk_tx_rate = NULL;
+
+		/* SGMII requires a PCS */
+		plat_dat->default_an_inband = true;
+		plat_dat->pcs_init = rk_pcs_init;
+		plat_dat->pcs_exit = rk_pcs_exit;
+		plat_dat->select_pcs = rk_select_pcs;
+	}
+
 	plat_dat->bsp_priv = rk_gmac_setup(pdev, plat_dat, data);
 	if (IS_ERR(plat_dat->bsp_priv))
 		return PTR_ERR(plat_dat->bsp_priv);
-- 
2.47.3

[PATCH net-next v10 6/6] MAINTAINERS: add entry for Rockchip XPCS driver

[Coia Prant]

Add a MAINTAINERS entry for the Rockchip RK3568 XPCS platform driver
and its device tree binding.

Include the relevant mailing lists (netdev and linux-rockchip) so that
future patches are properly distributed.

Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 65e8a4b5c90b1..2f0f46eabcf54 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23757,6 +23757,15 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/sound/rockchip,rk3576-sai.yaml
 F:	sound/soc/rockchip/rockchip_sai.*
 
+ROCKCHIP XPCS DRIVER
+M:	Coia Prant <coiaprant@gmail.com>
+L:	netdev@vger.kernel.org
+L:	linux-rockchip@lists.infradead.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
+F:	drivers/net/pcs/pcs-xpcs-rk.c
+F:	include/linux/pcs/pcs-xpcs-rk.h
+
 ROCKER DRIVER
 M:	Jiri Pirko <jiri@resnulli.us>
 L:	netdev@vger.kernel.org
-- 
2.47.3

Re: [PATCH net-next v10 1/6] dt-bindings: net: pcs: add rockchip,rk3568-xpcs support

[Rob Herring]

On Tue, Oct 06, 2026 at 06:30:03AM +0800, Coia Prant wrote:
> Add device tree binding documentation for the Synopsys DesignWare
> XPCS integrated on the Rockchip RK3568 SoC.
> 
> The XPCS is accessed over the APB3 bus and internally connected to
> a Naneng Combo SerDes PHY.  It supports 1000BASE-X, SGMII, and
> QSGMII modes, with four MII ports.
> 
> The four MII ports are described as ethernet-pcs-mii@N child nodes,
> consumed by the Rockchip XPCS glue driver later in this series.
> 
> phys and phy-names are required because dtbs_check only validates
> required properties for enabled nodes. The SerDes link is a board-level
> design choice (combphy1 on some boards, combphy2 on others), so these
> properties must be provided by the board device tree, not the SoC dtsi.
> 
> The CRU reset lines (SRST_XPCS*) are intentionally not described: no
> in-tree user requests them, and bring-up relies on the PD_PIPE power
> domain, the SerDes PHY and the in-IP soft reset. They can be added
> later as optional without breaking ABI.
> 
> Signed-off-by: Coia Prant <coiaprant@gmail.com>
> ---
>  .../net/pcs/rockchip,rk3568-xpcs.yaml         | 110 ++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
> new file mode 100644
> index 0000000000000..703fcff0e3f70
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/pcs/rockchip,rk3568-xpcs.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip RK3568 Synopsys DesignWare Ethernet PCS
> +
> +maintainers:
> +  - Coia Prant <coiaprant@gmail.com>
> +
> +description: |
> +  Rockchip RK3568 SoC integrates a Synopsys DesignWare Ethernet Physical
> +  Coding Sublayer (XPCS).
> +  The PCS provides an interface between the Media Access Control (MAC)
> +  and the Physical Medium Attachment (PMA) sublayer through a Media
> +  Independent Interface (GMII).
> +
> +  The XPCS is accessed over the APB3 bus and internally connected to a
> +  Naneng Combo SerDes PHY.
> +  It supports 1000BASE-X, SGMII and QSGMII modes.
> +
> +  The block contains four MII ports that can be individually enabled and
> +  routed to one of the Ethernet GMAC controllers via the pcs-handle
> +  property in the MAC device tree node.
> +
> +properties:
> +  compatible:
> +    const: rockchip,rk3568-xpcs
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  clocks:
> +    items:
> +      - description: APB3 bus interface clock (clk_csr_i), required for register access
> +      - description: EEE clock (clk_eee_i), required for Energy Efficient Ethernet operation
> +
> +  clock-names:
> +    items:
> +      - const: csr
> +      - const: eee
> +
> +  phys:
> +    maxItems: 1
> +
> +  phy-names:
> +    const: serdes

You don't really need phy-names if there is only 1 entry.

> +
> +  power-domains:
> +    maxItems: 1
> +
> +patternProperties:
> +  "^ethernet-pcs-mii@[0-3]$":
> +    type: object
> +    description:
> +      One of the four MII ports of the XPCS. The port is linked to an
> +      Ethernet MAC controller via the pcs-handle property in the MAC's
> +      device tree node.
> +
> +    properties:
> +      reg:
> +        description: MII port number.
> +        enum: [0, 1, 2, 3]
> +
> +    required:
> +      - reg

Why the child nodes? They don't contain anything.

Perhaps that's due to pcs-handle not supporting arg cells to pass the 
port number? That's about to change[1].

Rob

[1] https://github.com/devicetree-org/dt-schema/pull/198

Re: [PATCH net-next v10 4/6] dt-bindings: net: rockchip-dwmac: document pcs-handle

[Rob Herring]

On Tue, Oct 06, 2026 at 06:30:06AM +0800, Coia Prant wrote:
> The Rockchip GMAC binding needs to describe the PCS reference used by
> the SGMII support added later in this series. The property will be
> parsed by rk_pcs_init(), and a missing phandle fails the probe. Add it
> and require it when phy-mode is "sgmii" on rockchip,rk3568-gmac, the
> only SoC in this binding that has SGMII support.
> 
> Signed-off-by: Coia Prant <coiaprant@gmail.com>
> ---
>  .../bindings/net/rockchip-dwmac.yaml           | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> index 80c252845349c..bb7540e838033 100644
> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
> @@ -120,6 +120,12 @@ properties:
>      maximum: 0x7F
>      default: 0x10
>  
> +  pcs-handle:
> +    description:
> +      Specifies a reference to a node representing the PCS device
> +      connected to this GMAC. Required when phy-mode is "sgmii".
> +    maxItems: 1
> +
>    phy-supply:
>      description: PHY regulator
>  
> @@ -159,6 +165,18 @@ allOf:
>          clocks:
>            minItems: 5
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: rockchip,rk3568-gmac
> +        phy-mode:
> +          contains:
> +            const: sgmii

The 'if' will also be true if 'phy-mode' is not present. Probably not 
what you want? You need 'required: [ phy-mode ]'.

Rob

Re: [PATCH net-next v10 4/6] dt-bindings: net: rockchip-dwmac: document pcs-handle

[Coia Prant]

On October 6, 2026 9:48:02 PM GMT+08:00, Rob Herring <robh@kernel.org> wrote:
>On Tue, Oct 06, 2026 at 06:30:06AM +0800, Coia Prant wrote:
>> The Rockchip GMAC binding needs to describe the PCS reference used by
>> the SGMII support added later in this series. The property will be
>> parsed by rk_pcs_init(), and a missing phandle fails the probe. Add it
>> and require it when phy-mode is "sgmii" on rockchip,rk3568-gmac, the
>> only SoC in this binding that has SGMII support.
>> 
>> Signed-off-by: Coia Prant <coiaprant@gmail.com>
>> ---
>>  .../bindings/net/rockchip-dwmac.yaml           | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
>> index 80c252845349c..bb7540e838033 100644
>> --- a/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/rockchip-dwmac.yaml
>> @@ -120,6 +120,12 @@ properties:
>>      maximum: 0x7F
>>      default: 0x10
>>  
>> +  pcs-handle:
>> +    description:
>> +      Specifies a reference to a node representing the PCS device
>> +      connected to this GMAC. Required when phy-mode is "sgmii".
>> +    maxItems: 1
>> +
>>    phy-supply:
>>      description: PHY regulator
>>  
>> @@ -159,6 +165,18 @@ allOf:
>>          clocks:
>>            minItems: 5
>>  
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: rockchip,rk3568-gmac
>> +        phy-mode:
>> +          contains:
>> +            const: sgmii
>
>The 'if' will also be true if 'phy-mode' is not present. Probably not 
>what you want? You need 'required: [ phy-mode ]'.
>
>Rob

Hi Rob,

Right, thanks. Without 'required: [ phy-mode ]' the 'if' matches whenever
the compatible matches, even if phy-mode is absent. I'll add it and send
a respin.

Thanks,
Coia

Re: [PATCH net-next v10 1/6] dt-bindings: net: pcs: add rockchip,rk3568-xpcs support

[Coia Prant]

On October 6, 2026 9:24:28 PM GMT+08:00, Rob Herring <robh@kernel.org> wrote:
>On Tue, Oct 06, 2026 at 06:30:03AM +0800, Coia Prant wrote:
>> Add device tree binding documentation for the Synopsys DesignWare
>> XPCS integrated on the Rockchip RK3568 SoC.
>> 
>> The XPCS is accessed over the APB3 bus and internally connected to
>> a Naneng Combo SerDes PHY.  It supports 1000BASE-X, SGMII, and
>> QSGMII modes, with four MII ports.
>> 
>> The four MII ports are described as ethernet-pcs-mii@N child nodes,
>> consumed by the Rockchip XPCS glue driver later in this series.
>> 
>> phys and phy-names are required because dtbs_check only validates
>> required properties for enabled nodes. The SerDes link is a board-level
>> design choice (combphy1 on some boards, combphy2 on others), so these
>> properties must be provided by the board device tree, not the SoC dtsi.
>> 
>> The CRU reset lines (SRST_XPCS*) are intentionally not described: no
>> in-tree user requests them, and bring-up relies on the PD_PIPE power
>> domain, the SerDes PHY and the in-IP soft reset. They can be added
>> later as optional without breaking ABI.
>> 
>> Signed-off-by: Coia Prant <coiaprant@gmail.com>
>> ---
>>  .../net/pcs/rockchip,rk3568-xpcs.yaml         | 110 ++++++++++++++++++
>>  1 file changed, 110 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
>> new file mode 100644
>> index 0000000000000..703fcff0e3f70
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
>> @@ -0,0 +1,110 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/pcs/rockchip,rk3568-xpcs.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Rockchip RK3568 Synopsys DesignWare Ethernet PCS
>> +
>> +maintainers:
>> +  - Coia Prant <coiaprant@gmail.com>
>> +
>> +description: |
>> +  Rockchip RK3568 SoC integrates a Synopsys DesignWare Ethernet Physical
>> +  Coding Sublayer (XPCS).
>> +  The PCS provides an interface between the Media Access Control (MAC)
>> +  and the Physical Medium Attachment (PMA) sublayer through a Media
>> +  Independent Interface (GMII).
>> +
>> +  The XPCS is accessed over the APB3 bus and internally connected to a
>> +  Naneng Combo SerDes PHY.
>> +  It supports 1000BASE-X, SGMII and QSGMII modes.
>> +
>> +  The block contains four MII ports that can be individually enabled and
>> +  routed to one of the Ethernet GMAC controllers via the pcs-handle
>> +  property in the MAC device tree node.
>> +
>> +properties:
>> +  compatible:
>> +    const: rockchip,rk3568-xpcs
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 0
>> +
>> +  clocks:
>> +    items:
>> +      - description: APB3 bus interface clock (clk_csr_i), required for register access
>> +      - description: EEE clock (clk_eee_i), required for Energy Efficient Ethernet operation
>> +
>> +  clock-names:
>> +    items:
>> +      - const: csr
>> +      - const: eee
>> +
>> +  phys:
>> +    maxItems: 1
>> +
>> +  phy-names:
>> +    const: serdes
>
>You don't really need phy-names if there is only 1 entry.
>
>> +
>> +  power-domains:
>> +    maxItems: 1
>> +
>> +patternProperties:
>> +  "^ethernet-pcs-mii@[0-3]$":
>> +    type: object
>> +    description:
>> +      One of the four MII ports of the XPCS. The port is linked to an
>> +      Ethernet MAC controller via the pcs-handle property in the MAC's
>> +      device tree node.
>> +
>> +    properties:
>> +      reg:
>> +        description: MII port number.
>> +        enum: [0, 1, 2, 3]
>> +
>> +    required:
>> +      - reg
>
>Why the child nodes? They don't contain anything.
>
>Perhaps that's due to pcs-handle not supporting arg cells to pass the 
>port number? That's about to change[1].
>
>Rob
>
>[1] https://github.com/devicetree-org/dt-schema/pull/198

Hi Rob,

Both points make sense.

1. I'll drop phy-names since there's only a single entry.

2. For the ethernet-pcs-mii child nodes: you're right that they only
   contain 'reg'. The reason I used child nodes is because pcs-handle
   arg cells are not available yet -- PR #198 is still open and in
   RFC/change-request state.

   The RZN1 MII converter binding does the same thing: it declares
   MII ports as subnodes and references the PCS via pcs-handle, until
   arg cells land.

   So I'd like to keep the child nodes as a temporary workaround, and
   I'll add a note in the binding that this can be simplified once
   PR #198 is merged.

Thanks,
Coia

Re: [PATCH net-next v10 1/6] dt-bindings: net: pcs: add rockchip,rk3568-xpcs support

[Rob Herring]

On Tue, Oct 06, 2026 at 09:59:49PM +0800, Coia Prant wrote:
> On October 6, 2026 9:24:28 PM GMT+08:00, Rob Herring <robh@kernel.org> wrote:
> >On Tue, Oct 06, 2026 at 06:30:03AM +0800, Coia Prant wrote:
> >> Add device tree binding documentation for the Synopsys DesignWare
> >> XPCS integrated on the Rockchip RK3568 SoC.
> >> 
> >> The XPCS is accessed over the APB3 bus and internally connected to
> >> a Naneng Combo SerDes PHY.  It supports 1000BASE-X, SGMII, and
> >> QSGMII modes, with four MII ports.
> >> 
> >> The four MII ports are described as ethernet-pcs-mii@N child nodes,
> >> consumed by the Rockchip XPCS glue driver later in this series.
> >> 
> >> phys and phy-names are required because dtbs_check only validates
> >> required properties for enabled nodes. The SerDes link is a board-level
> >> design choice (combphy1 on some boards, combphy2 on others), so these
> >> properties must be provided by the board device tree, not the SoC dtsi.
> >> 
> >> The CRU reset lines (SRST_XPCS*) are intentionally not described: no
> >> in-tree user requests them, and bring-up relies on the PD_PIPE power
> >> domain, the SerDes PHY and the in-IP soft reset. They can be added
> >> later as optional without breaking ABI.
> >> 
> >> Signed-off-by: Coia Prant <coiaprant@gmail.com>
> >> ---
> >>  .../net/pcs/rockchip,rk3568-xpcs.yaml         | 110 ++++++++++++++++++
> >>  1 file changed, 110 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
> >> 
> >> diff --git a/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
> >> new file mode 100644
> >> index 0000000000000..703fcff0e3f70
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
> >> @@ -0,0 +1,110 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/net/pcs/rockchip,rk3568-xpcs.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Rockchip RK3568 Synopsys DesignWare Ethernet PCS
> >> +
> >> +maintainers:
> >> +  - Coia Prant <coiaprant@gmail.com>
> >> +
> >> +description: |
> >> +  Rockchip RK3568 SoC integrates a Synopsys DesignWare Ethernet Physical
> >> +  Coding Sublayer (XPCS).
> >> +  The PCS provides an interface between the Media Access Control (MAC)
> >> +  and the Physical Medium Attachment (PMA) sublayer through a Media
> >> +  Independent Interface (GMII).
> >> +
> >> +  The XPCS is accessed over the APB3 bus and internally connected to a
> >> +  Naneng Combo SerDes PHY.
> >> +  It supports 1000BASE-X, SGMII and QSGMII modes.
> >> +
> >> +  The block contains four MII ports that can be individually enabled and
> >> +  routed to one of the Ethernet GMAC controllers via the pcs-handle
> >> +  property in the MAC device tree node.
> >> +
> >> +properties:
> >> +  compatible:
> >> +    const: rockchip,rk3568-xpcs
> >> +
> >> +  reg:
> >> +    maxItems: 1
> >> +
> >> +  "#address-cells":
> >> +    const: 1
> >> +
> >> +  "#size-cells":
> >> +    const: 0
> >> +
> >> +  clocks:
> >> +    items:
> >> +      - description: APB3 bus interface clock (clk_csr_i), required for register access
> >> +      - description: EEE clock (clk_eee_i), required for Energy Efficient Ethernet operation
> >> +
> >> +  clock-names:
> >> +    items:
> >> +      - const: csr
> >> +      - const: eee
> >> +
> >> +  phys:
> >> +    maxItems: 1
> >> +
> >> +  phy-names:
> >> +    const: serdes
> >
> >You don't really need phy-names if there is only 1 entry.
> >
> >> +
> >> +  power-domains:
> >> +    maxItems: 1
> >> +
> >> +patternProperties:
> >> +  "^ethernet-pcs-mii@[0-3]$":
> >> +    type: object
> >> +    description:
> >> +      One of the four MII ports of the XPCS. The port is linked to an
> >> +      Ethernet MAC controller via the pcs-handle property in the MAC's
> >> +      device tree node.
> >> +
> >> +    properties:
> >> +      reg:
> >> +        description: MII port number.
> >> +        enum: [0, 1, 2, 3]
> >> +
> >> +    required:
> >> +      - reg
> >
> >Why the child nodes? They don't contain anything.
> >
> >Perhaps that's due to pcs-handle not supporting arg cells to pass the 
> >port number? That's about to change[1].
> >
> >Rob
> >
> >[1] https://github.com/devicetree-org/dt-schema/pull/198
> 
> Hi Rob,
> 
> Both points make sense.
> 
> 1. I'll drop phy-names since there's only a single entry.
> 
> 2. For the ethernet-pcs-mii child nodes: you're right that they only
>    contain 'reg'. The reason I used child nodes is because pcs-handle
>    arg cells are not available yet -- PR #198 is still open and in
>    RFC/change-request state.
> 
>    The RZN1 MII converter binding does the same thing: it declares
>    MII ports as subnodes and references the PCS via pcs-handle, until
>    arg cells land.
> 
>    So I'd like to keep the child nodes as a temporary workaround, and
>    I'll add a note in the binding that this can be simplified once
>    PR #198 is merged.

Bindings are an ABI. You can't merge the binding then change it. Please 
comment on the PR that you all need it.

Rob

Re: [PATCH net-next v10 1/6] dt-bindings: net: pcs: add rockchip,rk3568-xpcs support

[Coia Prant]

On October 6, 2026 11:08:31 PM GMT+08:00, Rob Herring <robh@kernel.org> wrote:
>On Tue, Oct 06, 2026 at 09:59:49PM +0800, Coia Prant wrote:
>> On October 6, 2026 9:24:28 PM GMT+08:00, Rob Herring <robh@kernel.org> wrote:
>> >On Tue, Oct 06, 2026 at 06:30:03AM +0800, Coia Prant wrote:
>> >> Add device tree binding documentation for the Synopsys DesignWare
>> >> XPCS integrated on the Rockchip RK3568 SoC.
>> >> 
>> >> The XPCS is accessed over the APB3 bus and internally connected to
>> >> a Naneng Combo SerDes PHY.  It supports 1000BASE-X, SGMII, and
>> >> QSGMII modes, with four MII ports.
>> >> 
>> >> The four MII ports are described as ethernet-pcs-mii@N child nodes,
>> >> consumed by the Rockchip XPCS glue driver later in this series.
>> >> 
>> >> phys and phy-names are required because dtbs_check only validates
>> >> required properties for enabled nodes. The SerDes link is a board-level
>> >> design choice (combphy1 on some boards, combphy2 on others), so these
>> >> properties must be provided by the board device tree, not the SoC dtsi.
>> >> 
>> >> The CRU reset lines (SRST_XPCS*) are intentionally not described: no
>> >> in-tree user requests them, and bring-up relies on the PD_PIPE power
>> >> domain, the SerDes PHY and the in-IP soft reset. They can be added
>> >> later as optional without breaking ABI.
>> >> 
>> >> Signed-off-by: Coia Prant <coiaprant@gmail.com>
>> >> ---
>> >>  .../net/pcs/rockchip,rk3568-xpcs.yaml         | 110 ++++++++++++++++++
>> >>  1 file changed, 110 insertions(+)
>> >>  create mode 100644 Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
>> >> 
>> >> diff --git a/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
>> >> new file mode 100644
>> >> index 0000000000000..703fcff0e3f70
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
>> >> @@ -0,0 +1,110 @@
>> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> >> +%YAML 1.2
>> >> +---
>> >> +$id: http://devicetree.org/schemas/net/pcs/rockchip,rk3568-xpcs.yaml#
>> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> >> +
>> >> +title: Rockchip RK3568 Synopsys DesignWare Ethernet PCS
>> >> +
>> >> +maintainers:
>> >> +  - Coia Prant <coiaprant@gmail.com>
>> >> +
>> >> +description: |
>> >> +  Rockchip RK3568 SoC integrates a Synopsys DesignWare Ethernet Physical
>> >> +  Coding Sublayer (XPCS).
>> >> +  The PCS provides an interface between the Media Access Control (MAC)
>> >> +  and the Physical Medium Attachment (PMA) sublayer through a Media
>> >> +  Independent Interface (GMII).
>> >> +
>> >> +  The XPCS is accessed over the APB3 bus and internally connected to a
>> >> +  Naneng Combo SerDes PHY.
>> >> +  It supports 1000BASE-X, SGMII and QSGMII modes.
>> >> +
>> >> +  The block contains four MII ports that can be individually enabled and
>> >> +  routed to one of the Ethernet GMAC controllers via the pcs-handle
>> >> +  property in the MAC device tree node.
>> >> +
>> >> +properties:
>> >> +  compatible:
>> >> +    const: rockchip,rk3568-xpcs
>> >> +
>> >> +  reg:
>> >> +    maxItems: 1
>> >> +
>> >> +  "#address-cells":
>> >> +    const: 1
>> >> +
>> >> +  "#size-cells":
>> >> +    const: 0
>> >> +
>> >> +  clocks:
>> >> +    items:
>> >> +      - description: APB3 bus interface clock (clk_csr_i), required for register access
>> >> +      - description: EEE clock (clk_eee_i), required for Energy Efficient Ethernet operation
>> >> +
>> >> +  clock-names:
>> >> +    items:
>> >> +      - const: csr
>> >> +      - const: eee
>> >> +
>> >> +  phys:
>> >> +    maxItems: 1
>> >> +
>> >> +  phy-names:
>> >> +    const: serdes
>> >
>> >You don't really need phy-names if there is only 1 entry.
>> >
>> >> +
>> >> +  power-domains:
>> >> +    maxItems: 1
>> >> +
>> >> +patternProperties:
>> >> +  "^ethernet-pcs-mii@[0-3]$":
>> >> +    type: object
>> >> +    description:
>> >> +      One of the four MII ports of the XPCS. The port is linked to an
>> >> +      Ethernet MAC controller via the pcs-handle property in the MAC's
>> >> +      device tree node.
>> >> +
>> >> +    properties:
>> >> +      reg:
>> >> +        description: MII port number.
>> >> +        enum: [0, 1, 2, 3]
>> >> +
>> >> +    required:
>> >> +      - reg
>> >
>> >Why the child nodes? They don't contain anything.
>> >
>> >Perhaps that's due to pcs-handle not supporting arg cells to pass the 
>> >port number? That's about to change[1].
>> >
>> >Rob
>> >
>> >[1] https://github.com/devicetree-org/dt-schema/pull/198
>> 
>> Hi Rob,
>> 
>> Both points make sense.
>> 
>> 1. I'll drop phy-names since there's only a single entry.
>> 
>> 2. For the ethernet-pcs-mii child nodes: you're right that they only
>>    contain 'reg'. The reason I used child nodes is because pcs-handle
>>    arg cells are not available yet -- PR #198 is still open and in
>>    RFC/change-request state.
>> 
>>    The RZN1 MII converter binding does the same thing: it declares
>>    MII ports as subnodes and references the PCS via pcs-handle, until
>>    arg cells land.
>> 
>>    So I'd like to keep the child nodes as a temporary workaround, and
>>    I'll add a note in the binding that this can be simplified once
>>    PR #198 is merged.
>
>Bindings are an ABI. You can't merge the binding then change it. Please 
>comment on the PR that you all need it.
>
>Rob

Hi Rob,

Understood on the ABI point, and I don't want to merge a binding we'd
have to change later.

Could I ask for your guidance on the practical path? This series is
ready, and I'd like to get it into 7.4 if possible, since OpenWrt and
other distros base their support on LTS kernels. Missing this window
means a long wait for users.

Given PR #198 is still open, I see these options:

1. Wait for PR #198, then use pcs-handle = <&xpcs 0>. My concern is that
   I have no visibility into its timeline -- it could be weeks or much
   longer -- and holding the whole binding on that is hard to plan
   around.

2. Keep the child nodes as the final ABI, RZ/N1 style, no planned
   migration.

3. Something else you'd prefer.

Which would you recommend? If waiting is the right call, I'll do that,
but I'd like to understand roughly how long PR #198 is expected to take.

Thanks,
Coia

Re: [PATCH net-next v10 5/6] net: stmmac: dwmac-rk: add SGMII support for RK3568

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: stmmac: memory leak and SGMII breakage due to xpcs overwrite
--

commit 9f6d3f852317e81db5e938c043afc40343ece6e9
Author: Coia Prant <coiaprant@gmail.com>

net: stmmac: dwmac-rk: add SGMII support for RK3568

This commit adds SGMII support for the RK3568 SoC to the dwmac-rk driver.
It configures the necessary glue logic, sets up the MAC for in-band
negotiation, and provides callbacks to initialize and manage an XPCS via
the Rockchip platform driver.

Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index 72bdbcb5e863a..3b50ad3c600b9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c

[ ... ]

> @@ -1211,6 +1239,43 @@ static void rk_phy_powerdown(struct rk_priv_data *bsp_priv)
> +static int rk_pcs_init(struct stmmac_priv *priv)
> +{
> +	struct device_node *np = priv->device->of_node;
> +	struct device_node *pcs_node;
> +	struct dw_xpcs *xpcs;
> +
> +	pcs_node = of_parse_phandle(np, "pcs-handle", 0);
> +	if (!pcs_node)
> +		return -ENODEV;
> +
> +	xpcs = xpcs_rk_create(priv->device, pcs_node);
> +	of_node_put(pcs_node);
> +	if (IS_ERR(xpcs))
> +		return PTR_ERR(xpcs);
> +
> +	priv->hw->xpcs = xpcs;

[Severity: High]
Does assigning priv->hw->xpcs here leak the allocated memory and break
SGMII initialization?

Looking at stmmac_pcs_setup() in
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c, the local xpcs variable
initializes to NULL and remains NULL when priv->plat->pcs_init() is called:

stmmac_mdio.c:stmmac_pcs_setup() {
    struct dw_xpcs *xpcs = NULL;
    ...
    if (priv->plat->pcs_init) {
        ret = priv->plat->pcs_init(priv);
    } else if (fwnode_property_present(devnode, "pcs-handle")) {
        ...
    }
    ...
    priv->hw->xpcs = xpcs;
}

Because rk_pcs_init() successfully allocates an XPCS and assigns it to
priv->hw->xpcs, the caller will immediately overwrite priv->hw->xpcs with
its local NULL variable. This drops the reference to the newly allocated
dw_xpcs structure and leaves the device without a valid XPCS, which breaks
rk_select_pcs() and the SGMII configuration.

Should rk_pcs_init() be updated to return the xpcs pointer, or does the
core stmmac_pcs_setup() logic need to be modified to avoid overwriting the
platform-allocated xpcs pointer?

> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005223011.1124347-1-coiaprant@gmail.com?part=5

Re: [PATCH net-next v10 1/6] dt-bindings: net: pcs: add rockchip,rk3568-xpcs support

[Coia Prant]

Coia Prant <coiaprant@gmail.com> 于2026年10月6日周二 23:52写道：
>
> On October 6, 2026 11:08:31 PM GMT+08:00, Rob Herring <robh@kernel.org> wrote:
> >On Tue, Oct 06, 2026 at 09:59:49PM +0800, Coia Prant wrote:
> >> On October 6, 2026 9:24:28 PM GMT+08:00, Rob Herring <robh@kernel.org> wrote:
> >> >On Tue, Oct 06, 2026 at 06:30:03AM +0800, Coia Prant wrote:
> >> >> Add device tree binding documentation for the Synopsys DesignWare
> >> >> XPCS integrated on the Rockchip RK3568 SoC.
> >> >>
> >> >> The XPCS is accessed over the APB3 bus and internally connected to
> >> >> a Naneng Combo SerDes PHY.  It supports 1000BASE-X, SGMII, and
> >> >> QSGMII modes, with four MII ports.
> >> >>
> >> >> The four MII ports are described as ethernet-pcs-mii@N child nodes,
> >> >> consumed by the Rockchip XPCS glue driver later in this series.
> >> >>
> >> >> phys and phy-names are required because dtbs_check only validates
> >> >> required properties for enabled nodes. The SerDes link is a board-level
> >> >> design choice (combphy1 on some boards, combphy2 on others), so these
> >> >> properties must be provided by the board device tree, not the SoC dtsi.
> >> >>
> >> >> The CRU reset lines (SRST_XPCS*) are intentionally not described: no
> >> >> in-tree user requests them, and bring-up relies on the PD_PIPE power
> >> >> domain, the SerDes PHY and the in-IP soft reset. They can be added
> >> >> later as optional without breaking ABI.
> >> >>
> >> >> Signed-off-by: Coia Prant <coiaprant@gmail.com>
> >> >> ---
> >> >>  .../net/pcs/rockchip,rk3568-xpcs.yaml         | 110 ++++++++++++++++++
> >> >>  1 file changed, 110 insertions(+)
> >> >>  create mode 100644 Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
> >> >> new file mode 100644
> >> >> index 0000000000000..703fcff0e3f70
> >> >> --- /dev/null
> >> >> +++ b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
> >> >> @@ -0,0 +1,110 @@
> >> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> >> +%YAML 1.2
> >> >> +---
> >> >> +$id: http://devicetree.org/schemas/net/pcs/rockchip,rk3568-xpcs.yaml#
> >> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> >> +
> >> >> +title: Rockchip RK3568 Synopsys DesignWare Ethernet PCS
> >> >> +
> >> >> +maintainers:
> >> >> +  - Coia Prant <coiaprant@gmail.com>
> >> >> +
> >> >> +description: |
> >> >> +  Rockchip RK3568 SoC integrates a Synopsys DesignWare Ethernet Physical
> >> >> +  Coding Sublayer (XPCS).
> >> >> +  The PCS provides an interface between the Media Access Control (MAC)
> >> >> +  and the Physical Medium Attachment (PMA) sublayer through a Media
> >> >> +  Independent Interface (GMII).
> >> >> +
> >> >> +  The XPCS is accessed over the APB3 bus and internally connected to a
> >> >> +  Naneng Combo SerDes PHY.
> >> >> +  It supports 1000BASE-X, SGMII and QSGMII modes.
> >> >> +
> >> >> +  The block contains four MII ports that can be individually enabled and
> >> >> +  routed to one of the Ethernet GMAC controllers via the pcs-handle
> >> >> +  property in the MAC device tree node.
> >> >> +
> >> >> +properties:
> >> >> +  compatible:
> >> >> +    const: rockchip,rk3568-xpcs
> >> >> +
> >> >> +  reg:
> >> >> +    maxItems: 1
> >> >> +
> >> >> +  "#address-cells":
> >> >> +    const: 1
> >> >> +
> >> >> +  "#size-cells":
> >> >> +    const: 0
> >> >> +
> >> >> +  clocks:
> >> >> +    items:
> >> >> +      - description: APB3 bus interface clock (clk_csr_i), required for register access
> >> >> +      - description: EEE clock (clk_eee_i), required for Energy Efficient Ethernet operation
> >> >> +
> >> >> +  clock-names:
> >> >> +    items:
> >> >> +      - const: csr
> >> >> +      - const: eee
> >> >> +
> >> >> +  phys:
> >> >> +    maxItems: 1
> >> >> +
> >> >> +  phy-names:
> >> >> +    const: serdes
> >> >
> >> >You don't really need phy-names if there is only 1 entry.
> >> >
> >> >> +
> >> >> +  power-domains:
> >> >> +    maxItems: 1
> >> >> +
> >> >> +patternProperties:
> >> >> +  "^ethernet-pcs-mii@[0-3]$":
> >> >> +    type: object
> >> >> +    description:
> >> >> +      One of the four MII ports of the XPCS. The port is linked to an
> >> >> +      Ethernet MAC controller via the pcs-handle property in the MAC's
> >> >> +      device tree node.
> >> >> +
> >> >> +    properties:
> >> >> +      reg:
> >> >> +        description: MII port number.
> >> >> +        enum: [0, 1, 2, 3]
> >> >> +
> >> >> +    required:
> >> >> +      - reg
> >> >
> >> >Why the child nodes? They don't contain anything.
> >> >
> >> >Perhaps that's due to pcs-handle not supporting arg cells to pass the
> >> >port number? That's about to change[1].
> >> >
> >> >Rob
> >> >
> >> >[1] https://github.com/devicetree-org/dt-schema/pull/198
> >>
> >> Hi Rob,
> >>
> >> Both points make sense.
> >>
> >> 1. I'll drop phy-names since there's only a single entry.
> >>
> >> 2. For the ethernet-pcs-mii child nodes: you're right that they only
> >>    contain 'reg'. The reason I used child nodes is because pcs-handle
> >>    arg cells are not available yet -- PR #198 is still open and in
> >>    RFC/change-request state.
> >>
> >>    The RZN1 MII converter binding does the same thing: it declares
> >>    MII ports as subnodes and references the PCS via pcs-handle, until
> >>    arg cells land.
> >>
> >>    So I'd like to keep the child nodes as a temporary workaround, and
> >>    I'll add a note in the binding that this can be simplified once
> >>    PR #198 is merged.
> >
> >Bindings are an ABI. You can't merge the binding then change it. Please
> >comment on the PR that you all need it.
> >
> >Rob
>
> Hi Rob,
>
> Understood on the ABI point, and I don't want to merge a binding we'd
> have to change later.
>
> Could I ask for your guidance on the practical path? This series is
> ready, and I'd like to get it into 7.4 if possible, since OpenWrt and
> other distros base their support on LTS kernels. Missing this window
> means a long wait for users.
>
> Given PR #198 is still open, I see these options:
>
> 1. Wait for PR #198, then use pcs-handle = <&xpcs 0>. My concern is that
>    I have no visibility into its timeline -- it could be weeks or much
>    longer -- and holding the whole binding on that is hard to plan
>    around.
>
> 2. Keep the child nodes as the final ABI, RZ/N1 style, no planned
>    migration.
>
> 3. Something else you'd prefer.
>
> Which would you recommend? If waiting is the right call, I'll do that,
> but I'd like to understand roughly how long PR #198 is expected to take.
>
> Thanks,
> Coia

Hi Rob,

Thanks for the feedback. I went back to the Rockchip OEM SDK to check how
this is actually wired up, and it confirms the MII port is not an
independent board-level configuration -- it follows the MAC the SGMII
link is routed to (GMAC0 -> port 0, GMAC1 -> port 1).

So I've dropped the ethernet-pcs-mii child nodes and the phy-names
property, and the glue driver now derives the MII port from the MAC id.
The binding no longer needs pcs-handle argument cells, so we don't have
to wait for PR #198.

I'll post the updated series on the 9th.

Thanks,
Coia

Re: [PATCH net-next v10 1/6] dt-bindings: net: pcs: add rockchip,rk3568-xpcs support

[netdev-bot+sashiko]

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

Low priority / nit picks for your consideration:

- [Low] The new binding
  Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml…

--

dt-bindings: net: pcs: add rockchip,rk3568-xpcs support

This adds a new binding for the Synopsys DesignWare XPCS on the Rockchip
RK3568 SoC. The XPCS sits on the APB3 bus, connects to a Naneng Combo
SerDes PHY, and exposes four MII ports as ethernet-pcs-mii@N child nodes.
The commit message says the CRU reset lines are left out on purpose.

> diff --git a/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml
> new file mode 100644
> index 0000000000000..703fcff0e3f70
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/pcs/rockchip,rk3568-xpcs.yaml

[ ... ]

> +  phys:
> +    maxItems: 1
> +
> +  phy-names:
> +    const: serdes
> +
> +  power-domains:
> +    maxItems: 1
> +

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +  - clocks
> +  - clock-names
> +  - phys
> +  - phy-names
> +  - power-domains
> +
> +additionalProperties: false

[Severity: Low]
Should this binding describe the XPCS reset inputs? The CRU binding
header already has four reset IDs for this block:

include/dt-bindings/clock/rk3568-cru.h:
#define SRST_XPCS		150
#define SRST_XPCS_TX_DIV10	151
#define SRST_XPCS_RX_DIV10	152
#define SRST_XPCS_XGXS_RX	153

The commit message gives this reason for leaving them out:

  The CRU reset lines (SRST_XPCS*) are intentionally not described: no
  in-tree user requests them, and bring-up relies on the PD_PIPE power
  domain, the SerDes PHY and the in-IP soft reset.

That reason is based on what drivers use today, not on what the hardware
has. Documentation/devicetree/bindings/writing-bindings.rst says:

  DO attempt to make bindings complete even if a driver doesn't support
  some features.

The schema has no resets or reset-names property and ends with
additionalProperties: false. A DT that fully describes the hardware with
something like resets = <&cru SRST_XPCS>, ... would then fail
dt_binding_check and dtbs_check.

The in-IP soft reset also only acts inside the PCS register block. It
does not appear to cover the TX/RX div10 or XGXS RX reset domains.

Could resets and reset-names be added now, even as optional properties?

On a related point that I have not been able to confirm: the generic
snps,dw-xpcs binding documents an sbd_intr_o interrupts output, and
additionalProperties: false also rejects that here. Does RK3568 route
that interrupt to the GIC? If it does, should interrupts be listed too?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005223011.1124347-1-coiaprant%40gmail.com

Re: [PATCH net-next v10 2/6] net: pcs: xpcs: add ANRESTART support for SGMII link recovery

[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] Concern: the patch goes against the phylink_pcs_ops contract.
- [Low] Race between concurrent pcs_get_state() callers.

--

net: pcs: xpcs: add ANRESTART support for SGMII link recovery

This patch sets BMCR_ANRESTART together with BMCR_ANENABLE in
xpcs_config_aneg_c37_sgmii(), and clears DW_VR_MII_AN_INTR_STS there as
well. It also restarts AN from xpcs_get_state_c37_sgmii() when the link is
down and AN completion is latched. The goal is to recover the link on
RK3568 XPCS hardware, which does not restart SGMII AN by itself after a
disconnect.

> diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
> index b415b93d77c15..6466e0ff2a98b 100644
> --- a/drivers/net/pcs/pcs-xpcs.c
> +++ b/drivers/net/pcs/pcs-xpcs.c

[ ... ]

> @@ -816,9 +825,14 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
>  	if (ret < 0)
>  		return ret;
>
> +	/* Clear CL37 AN complete status */
> +	ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
>  		ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MII_BMCR,
> -				 mdio_ctrl | BMCR_ANENABLE);
> +				 mdio_ctrl | BMCR_ANENABLE | BMCR_ANRESTART);

[Severity: Low]
This is a small question about the phylink_pcs_ops contract, not a
functional regression. The pcs_config() kernel-doc in
include/linux/phylink.h says:

 * pcs_config() will be called when configuration of the PCS is required
 * or when the advertisement is possibly updated. It must not unnecessarily
 * disrupt an established link.

As far as I can tell, the existing code earlier in
xpcs_config_aneg_c37_sgmii() already clears BMCR_ANENABLE and sets it again
on every in-band call, and that alone restarts CL37 AN:

	if (mdio_ctrl & BMCR_ANENABLE) {
		ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MII_BMCR,
				 mdio_ctrl & ~BMCR_ANENABLE);

Returning a positive value from pcs_config() to request a restart is also
not an option for SGMII. phylink_pcs_an_restart() only calls
pcs_an_restart() when phy_interface_mode_is_8023z() is true.

So the added BMCR_ANRESTART should not disrupt an established link any more
than before. Is the extra bit expected to have an effect beyond the
existing ANENABLE toggle on RK3568? If so, would it help to say so in the
comment above?

>
>  	return ret;
>  }
> @@ -1093,9 +1107,18 @@ static int xpcs_get_state_c37_sgmii(struct dw_xpcs *xpcs,
>  		return 0;
>  	}
>
> -	/* Clear AN complete status or interrupt */
> -	if (state->an_complete)
> -		xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
> +	if (state->an_complete) {
> +		/* Clear AN complete status or interrupt */
> +		ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* Initiate the next round of AN */
> +		ret = xpcs_modify(xpcs, MDIO_MMD_VEND2, MII_BMCR, BMCR_ANRESTART,
> +				  BMCR_ANRESTART);
> +		if (ret < 0)
> +			return ret;
> +	}

[Severity: Low]
Can two pcs_get_state() callers running at the same time race on this
sequence?

xpcs_get_state_c37_sgmii() reads DW_VR_MII_AN_INTR_STS to compute
an_complete. It then clears the register and sets BMCR_ANRESTART, each in
its own MDIO transaction. The MDIO bus lock only covers one access at a
time.

Nothing serialises the callers against each other:

  phylink_resolve() -> phylink_mac_pcs_get_state()
    holds pl->state_mutex but not RTNL

  phylink_ethtool_ksettings_get() -> phylink_mac_pcs_get_state()
    holds only RTNL (MLO_AN_INBAND with no pl->phydev)

  phylink_mii_ioctl() -> phylink_mii_read() -> phylink_mac_pcs_get_state()
    holds only RTNL

One possible interleaving:

ethtool path                            phylink_resolve()
xpcs_get_state_c37_sgmii()
  read AN_INTR_STS: ANCMPLT, no link
  <delayed>
                                        xpcs_get_state_c37_sgmii()
                                          read AN_INTR_STS: ANCMPLT
                                          write AN_INTR_STS = 0
                                          set BMCR_ANRESTART
                                        new AN completes, link up
  write AN_INTR_STS = 0
  set BMCR_ANRESTART

Here, would the late write from the ethtool path discard the newly latched
ANCMPLT? Would its ANRESTART then abort the negotiation that just
succeeded?

The same interleaving looks possible against xpcs_config_aneg_c37_sgmii().
That function now also clears DW_VR_MII_AN_INTR_STS and sets
BMCR_ANENABLE | BMCR_ANRESTART while holding only state_mutex.

The commit message says:

    The latch is cleared before issuing the restart, not after: clearing it
    afterwards would discard a freshly latched ANCMPLT from the new
    negotiation.

Doesn't that reasoning assume a single caller? Before this patch, the clear
in get_state was not followed by a restart, so this race had no visible
effect.

With the restart, the result would be a spurious AN restart. That can delay
link-up, or briefly flap a link that was just reported up, until the next
negotiation completes.

>
>  	return 0;
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005223011.1124347-1-coiaprant%40gmail.com

Re: [PATCH net-next v10 3/6] net: pcs: xpcs: add Rockchip RK3568 platform glue driver

[netdev-bot+sashiko]

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

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

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

- [High] The system-suspend design described in the commit message and in
  the comment in xpcs_rk_system_suspend() does not work.
- [Medium] xpcs_rk_create() (drivers/net/pcs/pcs-xpcs-rk.c:537-560)
  decides it is safe to use the supplier's devm-managed drvdata by doing…
- [Medium] xpcs_rk_mdio_write_remapping() (drivers/net/pcs/pcs-xpcs-
  rk.c:141-147) sends writes to DW_VR_MII_AN_CTRL,…
- [Low] Baseline decision: false positive at the end of the series.

--

net: pcs: xpcs: add Rockchip RK3568 platform glue driver

This adds a platform glue driver for the Synopsys DesignWare XPCS in
RK3568. The driver exposes the XPCS through a virtual MDIO bus that maps
accesses onto APB3 registers, with each port remapped to its own MMD. It
also moves pcs-xpcs-plat.o into a new PCS_XPCS_PLATFORM module, exports
xpcs_rk_create() for MAC drivers, and marks the XPCS as part of the
wakeup path during system suspend to keep PD_PIPE powered for MAC WoL.

Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 59, CRU_CLKSEL_CON29)
Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part2%20V1.1-20210301.pdf (Page 2078)

> diff --git a/drivers/net/pcs/pcs-xpcs-rk.c b/drivers/net/pcs/pcs-xpcs-rk.c
> new file mode 100644
> index 0000000000000..35ee980a759e5
> --- /dev/null
> +++ b/drivers/net/pcs/pcs-xpcs-rk.c
> @@ -0,0 +1,619 @@

[ ... ]

> +static int xpcs_rk_mdio_write_remapping(int addr, int dev, int reg)
> +{
> +	switch (dev) {
> +	case MDIO_MMD_PMAPMD:
> +		return ROCKCHIP_MMD_PMAPMD;
> +	case MDIO_MMD_VEND2:
> +		break;
> +	default:
> +		return -ENXIO;
> +	}
> +
> +	/*
> +	 * These registers physically live only in MII (the management port).
> +	 * Ports 1-3 expose read-only mirrors of these bits, so writes must
> +	 * always target MII; the read path remaps per address and the
> +	 * hardware redirects to the port's mirror.
> +	 */
> +	switch (reg) {
> +	case DW_VR_MII_AN_CTRL:
> +	case DW_VR_MII_AN_INTR_STS:
> +	case DW_VR_MII_EEE_MCTRL0:
> +	case DW_VR_MII_EEE_MCTRL1:
> +	case DW_VR_MII_DIG_CTRL2:
> +		return ROCKCHIP_MMD_MII;

[Severity: Medium]

Suppose two GMACs are routed to different ports, for example GMAC0 on
mii@0 and GMAC1 on mii@1. The commit message says this is supported.
Do these registers then become shared state between two independent
dw_xpcs instances?

Writes from port 1 go to port 0's MMD 7. GMAC1's xpcs would then
program port 0's AN_CTRL and EEE_MCTRL0/1 and clear port 0's
AN_INTR_STS. One example is in xpcs_get_state_c37_sgmii():

    ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);

Another is in xpcs_config_eee():

    ret = xpcs_modify(xpcs, MDIO_MMD_VEND2, DW_VR_MII_EEE_MCTRL0, mask,

Could this change GMAC0's PCS mode or LPI settings, or clear its
AN-complete latch, without GMAC0's phylink instance knowing?

No lock spans more than one dw_xpcs instance. Split read-then-write
sequences on these registers can therefore race between the two
phylink instances. An example is the AN_INTR_STS read followed by a
write in xpcs_get_state_c37_1000basex().

There is a separate question if the per-port registers are not exact
mirrors of MII0. In that case reads come from port N's register while
writes go to MII0. Could port N's CL37_ANCMPLT_INTR latch then never be
cleared? If so, every xpcs_get_state_c37_sgmii() poll with the link down
would see an_complete and set BMCR_ANRESTART again.

> +	default:
> +		break;
> +	}

[ ... ]

> +	/*
> +	 * Establish the device link before reading the supplier's drvdata.
> +	 * device_link_add() does not fail on a supplier that is unbinding:
> +	 * it creates the link in DL_STATE_SUPPLIER_UNBIND. Whether the link
> +	 * actually protects the drvdata depends on the supplier's state at
> +	 * creation time.
> +	 *
> +	 * Check link->supplier->links.status right after creation. If the
> +	 * supplier was DL_DEV_DRIVER_BOUND, the link is in
> +	 * DL_STATE_CONSUMER_PROBE and device_links_unbind_consumers() will
> +	 * wait for this probe to finish before unbinding the supplier, so
> +	 * the drvdata stays valid for the rest of the function. Any other
> +	 * state means the supplier is not usable yet; defer and retry.
> +	 *
> +	 * The link is released automatically when the consumer device is
> +	 * destroyed (DL_FLAG_AUTOREMOVE_CONSUMER), so no explicit
> +	 * device_link_remove() is needed on the failure paths.
> +	 */
> +	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
> +	if (!link) {
> +		put_device(&pdev->dev);
> +		return ERR_PTR(-EPROBE_DEFER);
> +	}
> +
> +	if (READ_ONCE(link->supplier->links.status) != DL_DEV_DRIVER_BOUND) {
> +		put_device(&pdev->dev);
> +		return ERR_PTR(-EPROBE_DEFER);
> +	}

[Severity: Medium]

Does a bound supplier here guarantee that the link itself is in
DL_STATE_CONSUMER_PROBE?

device_link_init_status() creates the link as DL_STATE_DORMANT when the
supplier has no driver yet. fw_devlink doesn't parse pcs-handle, so the
GMAC can probe while the XPCS probe is still deferred on the combphy,
clocks or power domain:

CPU0 (GMAC probe)                 CPU1 (XPCS probe)
xpcs_rk_create()
  device_link_add()
    link is DL_STATE_DORMANT
                                  xpcs_rk_probe() completes
                                  device_links_driver_bound()
                                    link -> DL_STATE_AVAILABLE
                                    links.status -> DL_DEV_DRIVER_BOUND
  READ_ONCE(...links.status) passes
  pxpcs = platform_get_drvdata(pdev)
  xpcs_create_mdiodev(pxpcs->bus, ...)

Now suppose the XPCS is unbound through sysfs or rmmod.
device_links_unbind_consumers() waits only for CONSUMER_PROBE links:

    if (status == DL_STATE_CONSUMER_PROBE) {
        device_links_write_unlock();
        wait_for_device_probe();
        goto start;
    }
    WRITE_ONCE(link->status, DL_STATE_SUPPLIER_UNBIND);

For an AVAILABLE link it just continues, and devres frees pxpcs and the
mii_bus. Can CPU0 then dereference pxpcs->bus and pxpcs->eee_mult_fact
after they have been freed?

Even without an unbind, the GMAC probe would finish with this link
still in DL_STATE_AVAILABLE. Wouldn't device_links_driver_bound() for
the consumer then hit this check?

    WARN_ON(link->status != DL_STATE_CONSUMER_PROBE);

Also, the comment says the link is released when the consumer device is
destroyed. With DL_FLAG_AUTOREMOVE_CONSUMER, isn't it actually dropped
when the consumer's driver unbinds or its probe fails?

> +
> +	pxpcs = platform_get_drvdata(pdev);
> +	if (!pxpcs || !pxpcs->bus) {
> +		put_device(&pdev->dev);
> +		return ERR_PTR(-EPROBE_DEFER);
> +	}
> +
> +	xpcs = xpcs_create_mdiodev(pxpcs->bus, (int)port);

[ ... ]

> +static int xpcs_rk_system_suspend(struct device *dev)
> +{
> +	/*
> +	 * Keep the PD_PIPE power domain on during system suspend.
> +	 *
> +	 * PD_PIPE is shared with SATA/PCIe and would be powered down by
> +	 * genpd once all its consumers are suspended, killing the SerDes
> +	 * and breaking MAC WoL.  Mark the XPCS as part of the wakeup path
> +	 * so genpd keeps the domain on.  Unconditional because the XPCS
> +	 * core has no callback to convey the MAC WoL state.
> +	 */
> +	device_set_wakeup_path(dev);
> +	return 0;
> +}

[Severity: High]

Does device_set_wakeup_path() actually keep PD_PIPE powered here? The
commit message says:

  genpd then leaves the domain powered, because the Rockchip power
  domain driver sets GENPD_FLAG_ACTIVE_WAKEUP on PD_PIPE

In drivers/pmdomain/rockchip/pm-domains.c, though, PD_PIPE is declared
with active_wakeup set to false:

    [RK3568_PD_PIPE] = DOMAIN_RK3568("pipe", BIT(8), BIT(11), false, false),

rockchip_pm_add_one_domain() sets the flag only when that field is
true:

    pd->genpd.flags = GENPD_FLAG_PM_CLK | GENPD_FLAG_NO_STAY_ON;
    if (pd_info->active_wakeup)
        pd->genpd.flags |= GENPD_FLAG_ACTIVE_WAKEUP;

genpd_finish_suspend() skips the power-off only when both conditions
hold:

    if (device_awake_path(dev) && genpd_is_active_wakeup(genpd) &&
        !device_out_band_wakeup(dev))
        return 0;

So the XPCS is counted in suspended_count and genpd_sync_power_off()
runs. That function compares suspended_count with device_count and
checks sd_count. It does not look at the rpm_always_on that probe set
with dev_pm_genpd_rpm_always_on(dev, true).

Once the other PD_PIPE consumers have suspended, would the domain be
powered off during system suspend? That would leave SGMII WoL without
a working PCS.

If PD_PIPE does lose power, the XPCS register state is lost too.
xpcs_rk_system_resume() below is an empty stub and does not redo the
SerDes or XPCS bring-up. Does anything restore that state on resume?

Two things are not confirmed here: whether phylink fully reprograms the
XPCS on resume, and whether the combphy really sits in PD_PIPE. The
rk356x combphy DT nodes have no power-domains property.

> +
> +static int xpcs_rk_system_resume(struct device *dev)
> +{
> +	return 0;
> +}

[ ... ]

> diff --git a/include/linux/pcs/pcs-xpcs-rk.h b/include/linux/pcs/pcs-xpcs-rk.h
> new file mode 100644
> index 0000000000000..28723d5bd75cc
> --- /dev/null
> +++ b/include/linux/pcs/pcs-xpcs-rk.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __LINUX_PCS_XPCS_ROCKCHIP_H
> +#define __LINUX_PCS_XPCS_ROCKCHIP_H
> +
> +#include <linux/device.h>
> +#include <linux/of.h>
> +#include <linux/pcs/pcs-xpcs.h>
> +
> +struct dw_xpcs *xpcs_rk_create(struct device *dev, struct device_node *np);

[Severity: Low]

This isn't a bug once the whole series is applied. In this patch on its
own, though, xpcs_rk_create() has no caller, and the prototype has no
stub for CONFIG_PCS_XPCS_ROCKCHIP=n.

The later commit "net: stmmac: dwmac-rk: add SGMII support for RK3568"
handles both points:

- It adds rk_pcs_init(), which calls xpcs_rk_create().
- It wires rk_pcs_init() up through plat_dat->pcs_init, so
  stmmac_pcs_setup() never falls back to xpcs_create_fwnode() for this
  PCS.
- It makes DWMAC_ROCKCHIP select PCS_XPCS_ROCKCHIP, so the missing stub
  can't cause a link failure for that user.

Is a stub still wanted for future users that don't select the symbol?

> +
> +#endif /* __LINUX_PCS_XPCS_ROCKCHIP_H */

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005223011.1124347-1-coiaprant%40gmail.com

Re: [PATCH net-next v10 5/6] net: stmmac: dwmac-rk: add SGMII support for RK3568

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 3 (2) · Low: 0

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

- [High] SGMII cannot work at all, and the XPCS leaks on every probe.
- [Medium] With phy-mode="sgmii", rk_gmac_probe() clears
  plat_dat->set_clk_tx_rate and sets default_an_inband=true, but that…
- [Medium] The commit message's account of in-band vs out-of-band SGMII
  is wrong in three ways.
- [Medium] xpcs_rk_create(), which the new rk_pcs_init() calls, assumes
  that 'supplier->links.status == DL_DEV_DRIVER_BOUND' right after…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but
  rk_gmac_exit() assumes driver data already exists.
- [Medium] This problem wasn't introduced by this patch, but
  rk_gmac_resume() ignores the return value of rk_gmac_powerup() and…

--

net: stmmac: dwmac-rk: add SGMII support for RK3568

This patch adds RK3568 SGMII support to dwmac-rk. It adds a set_to_sgmii()
GRF callback, plus pcs_init/pcs_exit/select_pcs callbacks that create the
XPCS with xpcs_rk_create(). For SGMII it also enables clocks earlier in
rk_gmac_powerup(), sets default_an_inband and clears set_clk_tx_rate.

Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 386)

> On RK3568, the MAC clock is fixed at 125 MHz and cannot be dynamically
> changed by the stmmac core's set_clk_tx_rate callback.  In-band mode
> works because the PCS handles rate adaptation internally.  Out-of-band
> mode does not work because the MAC would need to change the clock rate
> to 125/12.5/1.25 MHz for 1000/100/10 Mbps respectively, and the clock
> is fixed.

[Severity: Medium]
Are these rates correct? The clk_tx_i kernel-doc in include/linux/stmmac.h
says "SGMII 125MHz SerDes clock divided by 1, 5, or 50". Also,
rgmii_clock(), which rk_set_clk_mac_speed() uses, returns
125/25/2.5 MHz.

The reason given for in-band working and out-of-band failing also doesn't
seem to match the code:

- rk_gmac_probe() sets set_clk_tx_rate to NULL for every SGMII
  configuration, so the MAC clock is handled the same way in both modes.
- The XPCS is told the speed in both modes: through MAC_AUTO_SW in
  xpcs_config_aneg_c37_sgmii() for in-band, and through a BMCR write in
  xpcs_link_up_sgmii_1000basex() for out-of-band.

> Enable default_an_inband for SGMII and disable the generic stmmac
> set_clk_tx_rate callback.

The callback that gets cleared is rk_set_clk_tx_rate(). That is the
Rockchip-specific one, which ends up in rk_set_clk_mac_speed() on rk3568,
not a generic stmmac callback.

Could the commit message be updated on these points?

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index 72bdbcb5e863a..3b50ad3c600b9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c

[ ... ]

> @@ -1211,6 +1239,43 @@ static void rk_phy_powerdown(struct rk_priv_data *bsp_priv)
>  		dev_err(bsp_priv->dev, "fail to disable phy-supply\n");
>  }
>  
> +static int rk_pcs_init(struct stmmac_priv *priv)
> +{
> +	struct device_node *np = priv->device->of_node;
> +	struct device_node *pcs_node;
> +	struct dw_xpcs *xpcs;
> +
> +	pcs_node = of_parse_phandle(np, "pcs-handle", 0);
> +	if (!pcs_node)
> +		return -ENODEV;
> +
> +	xpcs = xpcs_rk_create(priv->device, pcs_node);

[Severity: Medium]
Can xpcs_rk_create() race with the XPCS driver binding? It does this:

drivers/net/pcs/pcs-xpcs-rk.c:xpcs_rk_create() {
    ...
	link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
    ...
	if (READ_ONCE(link->supplier->links.status) != DL_DEV_DRIVER_BOUND) {
    ...
}

fw_devlink does not parse "pcs-handle", so nothing orders the supplier
ahead of this probe. Suppose the XPCS device has no driver when
device_link_add() runs:

- The link starts in DL_STATE_DORMANT.
- If the XPCS driver binds before the READ_ONCE() check,
  device_links_driver_bound() moves the link to DL_STATE_AVAILABLE, not
  DL_STATE_CONSUMER_PROBE.
- The DL_DEV_DRIVER_BOUND check still passes.

device_links_unbind_consumers() only waits for links in
DL_STATE_CONSUMER_PROBE. Could a concurrent unbind of the XPCS driver then
free the devm-managed pxpcs and mii_bus while xpcs_create_mdiodev() is
still using them?

Also, if the stmmac probe then succeeds, wouldn't
device_links_driver_bound() for the consumer hit
WARN_ON(link->status != DL_STATE_CONSUMER_PROBE)?

> +	of_node_put(pcs_node);
> +	if (IS_ERR(xpcs))
> +		return PTR_ERR(xpcs);
> +
> +	priv->hw->xpcs = xpcs;

[Severity: High]
Does this XPCS pointer survive after the callback returns? On the pcs_init
branch, stmmac_pcs_setup() never sets its local xpcs, and then writes it
back unconditionally:

drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c:stmmac_pcs_setup() {
	struct dw_xpcs *xpcs = NULL;
	...
	if (priv->plat->pcs_init) {
		ret = priv->plat->pcs_init(priv);
	} else if (fwnode_property_present(devnode, "pcs-handle")) {
	...
	priv->hw->xpcs = xpcs;

	return 0;
}

So once rk_pcs_init() returns, priv->hw->xpcs is NULL again.

In stmmac_phylink_setup(), priv->hw->xpcs and priv->hw->phylink_pcs are
then both NULL, so the XPCS supported_interfaces are never merged.
rk_get_interfaces() has already set the RGMII and RMII bits for rk3568,
so PHY_INTERFACE_MODE_SGMII never ends up in supported_interfaces.

With default_an_inband set, phylink_parse_mode() takes the in-band path
and phylink_validate() fails. Doesn't that make phylink_create(), and so
the whole probe, fail with -EINVAL on every phy-mode = "sgmii" board?

Even if probe got past that, rk_select_pcs() would return NULL. Also,
rk_pcs_exit() and stmmac_pcs_clean() both return early when
priv->hw->xpcs is NULL, so xpcs_destroy() never runs. Is the dw_xpcs,
together with its mdio_device reference and clocks, leaked on every probe
attempt?

The other pcs_init users (socfpga, rzn1, renesas) avoid this by storing
their PCS in priv->hw->phylink_pcs.

Because of this, the commit message statement that SGMII "comes from the
XPCS's own supported_interfaces, merged by stmmac_phylink_setup()" does
not currently hold.

> +	return 0;
> +}
> +
> +static void rk_pcs_exit(struct stmmac_priv *priv)
> +{
> +	if (!priv->hw->xpcs)
> +		return;
> +
> +	xpcs_destroy(priv->hw->xpcs);
> +	priv->hw->xpcs = NULL;
> +}
> +
> +static struct phylink_pcs *rk_select_pcs(struct stmmac_priv *priv,
> +					 phy_interface_t interface)
> +{
> +	if (!priv->hw->xpcs)
> +		return NULL;
> +
> +	return xpcs_to_phylink_pcs(priv->hw->xpcs);
> +}

[ ... ]

> @@ -1466,6 +1541,10 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
>  		bsp_priv->ops->integrated_phy_powerup(bsp_priv);
>  
>  	return 0;
> +
> +clk_disable:
> +	gmac_clk_enable(bsp_priv, false);
> +	return ret;
>  }

[Severity: Medium]
This isn't a bug introduced by this patch, but rk_gmac_resume() ignores
the error returned here:

drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c:rk_gmac_resume() {
	...
	if (!device_may_wakeup(dev))
		rk_gmac_powerup(bsp_priv);

	return 0;
}

If the clock enable, a GRF write or the phy regulator fails during
resume, rk_gmac_powerup() returns an error and the clocks are disabled.

Won't stmmac_resume() then go on to reinitialise the MAC with its clocks
off?

[ ... ]

> @@ -1605,6 +1684,17 @@ static int rk_gmac_probe(struct platform_device *pdev)
>  	plat_dat->suspend = rk_gmac_suspend;
>  	plat_dat->resume = rk_gmac_resume;
>  
> +	if (plat_dat->phy_interface == PHY_INTERFACE_MODE_SGMII) {
> +		/* SGMII clock always runs at 125 MHz */
> +		plat_dat->set_clk_tx_rate = NULL;
> +
> +		/* SGMII requires a PCS */
> +		plat_dat->default_an_inband = true;

[Severity: Medium]
default_an_inband only picks the default. Does anything stop phylink from
running this link out-of-band?

- A fixed-link node gives MLO_AN_FIXED.
- If the PHY reports only LINK_INBAND_DISABLE: xpcs_inband_caps() returns
  LINK_INBAND_DISABLE | LINK_INBAND_ENABLE for SGMII, so
  phylink_pcs_neg_mode() selects PHYLINK_PCS_NEG_OUTBAND.

rk_select_pcs() returns the XPCS whatever the neg_mode is. With
set_clk_tx_rate cleared, the MAC clock stays at 125 MHz.

According to the commit message, 10/100 Mbps would then have working TX
and failing RX. The link would still be reported up, and nothing would be
logged.

Could the driver reject this or warn about it, for example by checking the
neg_mode or limiting the MAC capabilities? Right now it is only documented
in the commit message.

> +		plat_dat->pcs_init = rk_pcs_init;
> +		plat_dat->pcs_exit = rk_pcs_exit;
> +		plat_dat->select_pcs = rk_select_pcs;
> +	}
> +
>  	plat_dat->bsp_priv = rk_gmac_setup(pdev, plat_dat, data);
>  	if (IS_ERR(plat_dat->bsp_priv))
>  		return PTR_ERR(plat_dat->bsp_priv);

[Severity: Medium]
This is a pre-existing issue, but rk_gmac_exit(), which is installed as
plat_dat->exit in this function, assumes the driver data has already been
set:

drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c:rk_gmac_exit() {
	struct stmmac_priv *priv = netdev_priv(dev_get_drvdata(dev));
	...
	if (priv->plat->phy_node && bsp_priv->integrated_phy)
	...
}

stmmac_dvr_probe() calls plat->init before __stmmac_dvr_probe(), and
calls plat->exit if anything fails afterwards.

Suppose __stmmac_dvr_probe() fails before dev_set_drvdata(), for example
because devm_alloc_etherdev_mqs() returns NULL. Would rk_gmac_exit() then
dereference a priv derived from a NULL net_device?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005223011.1124347-1-coiaprant%40gmail.com

Re: [PATCH net-next v10 5/6] net: stmmac: dwmac-rk: add SGMII support for RK3568

[Coia Prant]

<netdev-bot+sashiko@kernel.org> 于2026年10月9日周五 00:32写道：
>
> Thank you for your contribution! Sashiko AI review found 4 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 3 (2) · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] SGMII cannot work at all, and the XPCS leaks on every probe.
> - [Medium] With phy-mode="sgmii", rk_gmac_probe() clears
>   plat_dat->set_clk_tx_rate and sets default_an_inband=true, but that…
> - [Medium] The commit message's account of in-band vs out-of-band SGMII
>   is wrong in three ways.
> - [Medium] xpcs_rk_create(), which the new rk_pcs_init() calls, assumes
>   that 'supplier->links.status == DL_DEV_DRIVER_BOUND' right after…
>
> Pre-existing issues:
> - [Medium] This problem wasn't introduced by this patch, but
>   rk_gmac_exit() assumes driver data already exists.
> - [Medium] This problem wasn't introduced by this patch, but
>   rk_gmac_resume() ignores the return value of rk_gmac_powerup() and…
>
> --
>
> net: stmmac: dwmac-rk: add SGMII support for RK3568
>
> This patch adds RK3568 SGMII support to dwmac-rk. It adds a set_to_sgmii()
> GRF callback, plus pcs_init/pcs_exit/select_pcs callbacks that create the
> XPCS with xpcs_rk_create(). For SGMII it also enables clocks earlier in
> rk_gmac_powerup(), sets default_an_inband and clears set_clk_tx_rate.
>
> Link: https://dl.radxa.com/rock3/docs/hw/datasheet/Rockchip%20RK3568%20TRM%20Part1%20V1.1-20210301.pdf (Page 386)
>
> > On RK3568, the MAC clock is fixed at 125 MHz and cannot be dynamically
> > changed by the stmmac core's set_clk_tx_rate callback.  In-band mode
> > works because the PCS handles rate adaptation internally.  Out-of-band
> > mode does not work because the MAC would need to change the clock rate
> > to 125/12.5/1.25 MHz for 1000/100/10 Mbps respectively, and the clock
> > is fixed.
>
> [Severity: Medium]
> Are these rates correct? The clk_tx_i kernel-doc in include/linux/stmmac.h
> says "SGMII 125MHz SerDes clock divided by 1, 5, or 50". Also,
> rgmii_clock(), which rk_set_clk_mac_speed() uses, returns
> 125/25/2.5 MHz.
>
> The reason given for in-band working and out-of-band failing also doesn't
> seem to match the code:
>
> - rk_gmac_probe() sets set_clk_tx_rate to NULL for every SGMII
>   configuration, so the MAC clock is handled the same way in both modes.
> - The XPCS is told the speed in both modes: through MAC_AUTO_SW in
>   xpcs_config_aneg_c37_sgmii() for in-band, and through a BMCR write in
>   xpcs_link_up_sgmii_1000basex() for out-of-band.
>
> > Enable default_an_inband for SGMII and disable the generic stmmac
> > set_clk_tx_rate callback.
>
> The callback that gets cleared is rk_set_clk_tx_rate(). That is the
> Rockchip-specific one, which ends up in rk_set_clk_mac_speed() on rk3568,
> not a generic stmmac callback.
>
> Could the commit message be updated on these points?

Ack, I'll fix the commit message: the rates are 125/25/2.5 MHz, and the
reason out-of-band does not work is that the driver disables
set_clk_tx_rate, so the MAC clock stays at 125 MHz. It's a driver
choice, not a hardware limitation.

> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > index 72bdbcb5e863a..3b50ad3c600b9 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>
> [ ... ]
>
> > @@ -1211,6 +1239,43 @@ static void rk_phy_powerdown(struct rk_priv_data *bsp_priv)
> >               dev_err(bsp_priv->dev, "fail to disable phy-supply\n");
> >  }
> >
> > +static int rk_pcs_init(struct stmmac_priv *priv)
> > +{
> > +     struct device_node *np = priv->device->of_node;
> > +     struct device_node *pcs_node;
> > +     struct dw_xpcs *xpcs;
> > +
> > +     pcs_node = of_parse_phandle(np, "pcs-handle", 0);
> > +     if (!pcs_node)
> > +             return -ENODEV;
> > +
> > +     xpcs = xpcs_rk_create(priv->device, pcs_node);
>
> [Severity: Medium]
> Can xpcs_rk_create() race with the XPCS driver binding? It does this:
>
> drivers/net/pcs/pcs-xpcs-rk.c:xpcs_rk_create() {
>     ...
>         link = device_link_add(dev, &pdev->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
>     ...
>         if (READ_ONCE(link->supplier->links.status) != DL_DEV_DRIVER_BOUND) {
>     ...
> }
>
> fw_devlink does not parse "pcs-handle", so nothing orders the supplier
> ahead of this probe. Suppose the XPCS device has no driver when
> device_link_add() runs:
>
> - The link starts in DL_STATE_DORMANT.
> - If the XPCS driver binds before the READ_ONCE() check,
>   device_links_driver_bound() moves the link to DL_STATE_AVAILABLE, not
>   DL_STATE_CONSUMER_PROBE.
> - The DL_DEV_DRIVER_BOUND check still passes.
>
> device_links_unbind_consumers() only waits for links in
> DL_STATE_CONSUMER_PROBE. Could a concurrent unbind of the XPCS driver then
> free the devm-managed pxpcs and mii_bus while xpcs_create_mdiodev() is
> still using them?
>
> Also, if the stmmac probe then succeeds, wouldn't
> device_links_driver_bound() for the consumer hit
> WARN_ON(link->status != DL_STATE_CONSUMER_PROBE)?
>
> > +     of_node_put(pcs_node);
> > +     if (IS_ERR(xpcs))
> > +             return PTR_ERR(xpcs);
> > +
> > +     priv->hw->xpcs = xpcs;
>
> [Severity: High]
> Does this XPCS pointer survive after the callback returns? On the pcs_init
> branch, stmmac_pcs_setup() never sets its local xpcs, and then writes it
> back unconditionally:
>
> drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c:stmmac_pcs_setup() {
>         struct dw_xpcs *xpcs = NULL;
>         ...
>         if (priv->plat->pcs_init) {
>                 ret = priv->plat->pcs_init(priv);
>         } else if (fwnode_property_present(devnode, "pcs-handle")) {
>         ...
>         priv->hw->xpcs = xpcs;
>
>         return 0;
> }
>
> So once rk_pcs_init() returns, priv->hw->xpcs is NULL again.
>
> In stmmac_phylink_setup(), priv->hw->xpcs and priv->hw->phylink_pcs are
> then both NULL, so the XPCS supported_interfaces are never merged.
> rk_get_interfaces() has already set the RGMII and RMII bits for rk3568,
> so PHY_INTERFACE_MODE_SGMII never ends up in supported_interfaces.
>
> With default_an_inband set, phylink_parse_mode() takes the in-band path
> and phylink_validate() fails. Doesn't that make phylink_create(), and so
> the whole probe, fail with -EINVAL on every phy-mode = "sgmii" board?
>
> Even if probe got past that, rk_select_pcs() would return NULL. Also,
> rk_pcs_exit() and stmmac_pcs_clean() both return early when
> priv->hw->xpcs is NULL, so xpcs_destroy() never runs. Is the dw_xpcs,
> together with its mdio_device reference and clocks, leaked on every probe
> attempt?
>
> The other pcs_init users (socfpga, rzn1, renesas) avoid this by storing
> their PCS in priv->hw->phylink_pcs.
>
> Because of this, the commit message statement that SGMII "comes from the
> XPCS's own supported_interfaces, merged by stmmac_phylink_setup()" does
> not currently hold.

Fixed by the stmmac XPCS lifetime patch in the same series, which
rewrites stmmac_pcs_setup() and no longer overwrites priv->hw->xpcs with
a local variable. Sashiko is looking at the pre-patch baseline.

> [Severity: Medium]
> default_an_inband only picks the default. Does anything stop phylink from
> running this link out-of-band?
>
> - A fixed-link node gives MLO_AN_FIXED.
> - If the PHY reports only LINK_INBAND_DISABLE: xpcs_inband_caps() returns
>   LINK_INBAND_DISABLE | LINK_INBAND_ENABLE for SGMII, so
>   phylink_pcs_neg_mode() selects PHYLINK_PCS_NEG_OUTBAND.
>
> rk_select_pcs() returns the XPCS whatever the neg_mode is. With
> set_clk_tx_rate cleared, the MAC clock stays at 125 MHz.
>
> According to the commit message, 10/100 Mbps would then have working TX
> and failing RX. The link would still be reported up, and nothing would be
> logged.
>
> Could the driver reject this or warn about it, for example by checking the
> neg_mode or limiting the MAC capabilities? Right now it is only documented
> in the commit message.
>
> > +             plat_dat->pcs_init = rk_pcs_init;
> > +             plat_dat->pcs_exit = rk_pcs_exit;
> > +             plat_dat->select_pcs = rk_select_pcs;
> > +     }
> > +
> >       plat_dat->bsp_priv = rk_gmac_setup(pdev, plat_dat, data);
> >       if (IS_ERR(plat_dat->bsp_priv))
> >               return PTR_ERR(plat_dat->bsp_priv);

There is no platform hook to reject a neg_mode at the MAC level today;
pcs_init/select_pcs only give us a phylink_pcs. Limiting this would
need a stmmac/phylink core change, out of scope for this series. I've
documented the limitation in the commit message.