From 17822d5d184f3f733eb9fc8569ee404ea3c5f5bf Mon Sep 17 00:00:00 2001 From: Markus Stockhausen Date: Sun, 3 Aug 2025 11:05:36 -0400 Subject: [PATCH] realtek: use consistent definition in DTS for SFP(+) ports MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit We are slowly getting to the point where the mdio driver will be carved out from the ethernet driver. Since the beginning it had the feature to hand out SFP serdes as phys. So one can access them from the phy driver. This will be kept during the final migration and it even will provide a consistent interface for the phy/serdes registers. With this being done we need to identify how to handle the affected ports in a generic way for all targets. Doing first things first, this starts with a consistent DTS. Currently we have: for RTL838x + Zyxel XGS1210: phy-mode = "1000base-x" managed = "in-band-status" phy-handle = ... for all other RTL93x devices: phy-mode = "10gbase-r" managed = "in-band-status" pseudo-phy-handle = ... Looking at the phylink kernel code one can see a nifty detail. There is dynamic phy bringup depending on the mode. int phylink_fwnode_phy_connect(struct phylink *pl, const struct fwnode_handle *fwnode, u32 flags) { struct fwnode_handle *phy_fwnode; struct phy_device *phy_dev; int ret; /* Fixed links and 802.3z are handled without needing a PHY */ if (pl->cfg_link_an_mode == MLO_AN_FIXED || (pl->cfg_link_an_mode == MLO_AN_INBAND && phy_interface_mode_is_8023z(pl->link_interface))) return 0; ... } Where 802.3z means 1000base-x or 2500base-x. Aligning this with IEEE specs it means essentially: - 10gbase-r defined ports with phy-handle must statically bring up a phylink from the beginning that immediately depends on a phy read_status() implementation. - 1000base-x/2500base-x defined ports will dynamically bringup a phylink during link detection regardless of a phy-handle. So it usually runs at the moment when a SFP has been plugged in. We currently still rely on a phy-handle but do not want to bring up the phy immediately. Commit 4457c1eee49 ("realtek: rtl93xx: support SFPs with phys") tried to fix exactly that error for 10gbase-r definied ports. Kernel shows "sfp sfp-p8: sfp_add_phy failed: -EBUSY" in that case. But it did it in the wrong way. It implemented a workaround by introducing a DTS property "pseudo-phy-handle". Instead it should have simply converted the DTS nodes to 1000base-x. Revert the commit and fix the DTS with wrong definitions. From now on we have a consistent SFP definition throughout all DTS and targets. Aside from the positive effect this setting has it is more or less an arbitrary speed definition. When plugging in the SFP the real speed will be choosen dynamically. Fixes: 4457c1eee49 ("realtek: rtl93xx: support SFPs with phys") Tested-By: Bjørn Mork Signed-off-by: Markus Stockhausen Link: https://github.com/openwrt/openwrt/pull/19648 Signed-off-by: Hauke Mehrtens --- .../realtek/dts/rtl9302_zyxel_xgs1210-12.dts | 6 ++-- .../realtek/dts/rtl9302_zyxel_xgs1250-12.dts | 2 +- .../dts/rtl9303_tplink_tl-st1008f_v2.dts | 32 +++++++++---------- .../dts/rtl9303_vimin_vm-s100-0800ms.dts | 32 +++++++++---------- .../dts/rtl9303_xikestor_sks8300-8x.dts | 32 +++++++++---------- .../drivers/net/dsa/rtl83xx/common.c | 16 ---------- 6 files changed, 52 insertions(+), 68 deletions(-) diff --git a/target/linux/realtek/dts/rtl9302_zyxel_xgs1210-12.dts b/target/linux/realtek/dts/rtl9302_zyxel_xgs1210-12.dts index 44a5c260907..65954e3587b 100644 --- a/target/linux/realtek/dts/rtl9302_zyxel_xgs1210-12.dts +++ b/target/linux/realtek/dts/rtl9302_zyxel_xgs1210-12.dts @@ -327,8 +327,8 @@ port@26 { reg = <26>; label = "lan11"; - phy-mode = "1000base-x"; //"10gbase-r"; - pseudo-phy-handle = <&phy26>; + phy-mode = "1000base-x"; + phy-handle = <&phy26>; sfp = <&sfp0>; led-set = <2>; managed = "in-band-status"; @@ -338,7 +338,7 @@ reg = <27>; label = "lan12"; phy-mode = "1000base-x"; - pseudo-phy-handle = <&phy27>; + phy-handle = <&phy27>; sfp = <&sfp1>; led-set = <2>; managed = "in-band-status"; diff --git a/target/linux/realtek/dts/rtl9302_zyxel_xgs1250-12.dts b/target/linux/realtek/dts/rtl9302_zyxel_xgs1250-12.dts index ec1597a3b21..a2486f134e7 100644 --- a/target/linux/realtek/dts/rtl9302_zyxel_xgs1250-12.dts +++ b/target/linux/realtek/dts/rtl9302_zyxel_xgs1250-12.dts @@ -397,7 +397,7 @@ reg = <27>; label = "lan12"; phy-mode = "1000base-x"; - pseudo-phy-handle = <&phy27>; + phy-handle = <&phy27>; sfp = <&sfp0>; led-set = <2>; managed = "in-band-status"; diff --git a/target/linux/realtek/dts/rtl9303_tplink_tl-st1008f_v2.dts b/target/linux/realtek/dts/rtl9303_tplink_tl-st1008f_v2.dts index 4b8423a4d5d..75633e53f98 100644 --- a/target/linux/realtek/dts/rtl9303_tplink_tl-st1008f_v2.dts +++ b/target/linux/realtek/dts/rtl9303_tplink_tl-st1008f_v2.dts @@ -293,8 +293,8 @@ port@0 { reg = <0>; label = "lan1"; - phy-mode = "10gbase-r"; - pseudo-phy-handle = <&phy0>; + phy-mode = "1000base-x"; + phy-handle = <&phy0>; sfp = <&sfp0>; managed = "in-band-status"; led-set = <0>; @@ -303,8 +303,8 @@ port@8 { reg = <8>; label = "lan2"; - phy-mode = "10gbase-r"; - pseudo-phy-handle = <&phy8>; + phy-mode = "1000base-x"; + phy-handle = <&phy8>; sfp = <&sfp1>; managed = "in-band-status"; led-set = <0>; @@ -313,8 +313,8 @@ port@10 { reg = <16>; label = "lan3"; - phy-mode = "10gbase-r"; - pseudo-phy-handle = <&phy16>; + phy-mode = "1000base-x"; + phy-handle = <&phy16>; sfp = <&sfp2>; managed = "in-band-status"; led-set = <0>; @@ -323,8 +323,8 @@ port@14 { reg = <20>; label = "lan4"; - phy-mode = "10gbase-r"; - pseudo-phy-handle = <&phy20>; + phy-mode = "1000base-x"; + phy-handle = <&phy20>; sfp = <&sfp3>; managed = "in-band-status"; led-set = <0>; @@ -333,8 +333,8 @@ port@18 { reg = <24>; label = "lan5"; - phy-mode = "10gbase-r"; - pseudo-phy-handle = <&phy24>; + phy-mode = "1000base-x"; + phy-handle = <&phy24>; sfp = <&sfp4>; managed = "in-band-status"; led-set = <0>; @@ -343,8 +343,8 @@ port@19 { reg = <25>; label = "lan6"; - phy-mode = "10gbase-r"; - pseudo-phy-handle = <&phy25>; + phy-mode = "1000base-x"; + phy-handle = <&phy25>; sfp = <&sfp5>; managed = "in-band-status"; led-set = <0>; @@ -353,8 +353,8 @@ port@1a { reg = <26>; label = "lan7"; - phy-mode = "10gbase-r"; - pseudo-phy-handle = <&phy26>; + phy-mode = "1000base-x"; + phy-handle = <&phy26>; sfp = <&sfp6>; managed = "in-band-status"; led-set = <0>; @@ -363,8 +363,8 @@ port@1b { reg = <27>; label = "lan8"; - phy-mode = "10gbase-r"; - pseudo-phy-handle = <&phy27>; + phy-mode = "1000base-x"; + phy-handle = <&phy27>; sfp = <&sfp7>; managed = "in-band-status"; led-set = <0>; diff --git a/target/linux/realtek/dts/rtl9303_vimin_vm-s100-0800ms.dts b/target/linux/realtek/dts/rtl9303_vimin_vm-s100-0800ms.dts index 93f60d016dc..d901967a22a 100644 --- a/target/linux/realtek/dts/rtl9303_vimin_vm-s100-0800ms.dts +++ b/target/linux/realtek/dts/rtl9303_vimin_vm-s100-0800ms.dts @@ -282,8 +282,8 @@ port@0 { reg = <0>; label = "lan1"; - pseudo-phy-handle = <&phy0>; - phy-mode = "10gbase-r"; + phy-handle = <&phy0>; + phy-mode = "1000base-x"; sfp = <&sfp0>; managed = "in-band-status"; led-set = <0>; @@ -292,8 +292,8 @@ port@8 { reg = <8>; label = "lan2"; - pseudo-phy-handle = <&phy8>; - phy-mode = "10gbase-r"; + phy-handle = <&phy8>; + phy-mode = "1000base-x"; sfp = <&sfp1>; managed = "in-band-status"; led-set = <0>; @@ -302,8 +302,8 @@ port@10 { reg = <16>; label = "lan3"; - pseudo-phy-handle = <&phy16>; - phy-mode = "10gbase-r"; + phy-handle = <&phy16>; + phy-mode = "1000base-x"; sfp = <&sfp2>; managed = "in-band-status"; led-set = <0>; @@ -312,8 +312,8 @@ port@14 { reg = <20>; label = "lan4"; - pseudo-phy-handle = <&phy20>; - phy-mode = "10gbase-r"; + phy-handle = <&phy20>; + phy-mode = "1000base-x"; sfp = <&sfp3>; managed = "in-band-status"; led-set = <0>; @@ -322,8 +322,8 @@ port@18 { reg = <24>; label = "lan5"; - pseudo-phy-handle = <&phy24>; - phy-mode = "10gbase-r"; + phy-handle = <&phy24>; + phy-mode = "1000base-x"; sfp = <&sfp4>; managed = "in-band-status"; led-set = <0>; @@ -332,8 +332,8 @@ port@19 { reg = <25>; label = "lan6"; - pseudo-phy-handle = <&phy25>; - phy-mode = "10gbase-r"; + phy-handle = <&phy25>; + phy-mode = "1000base-x"; sfp = <&sfp5>; managed = "in-band-status"; led-set = <0>; @@ -342,8 +342,8 @@ port@1a { reg = <26>; label = "lan7"; - pseudo-phy-handle = <&phy26>; - phy-mode = "10gbase-r"; + phy-handle = <&phy26>; + phy-mode = "1000base-x"; sfp = <&sfp6>; managed = "in-band-status"; led-set = <0>; @@ -352,8 +352,8 @@ port@1b { reg = <27>; label = "lan8"; - pseudo-phy-handle = <&phy27>; - phy-mode = "10gbase-r"; + phy-handle = <&phy27>; + phy-mode = "1000base-x"; sfp = <&sfp7>; managed = "in-band-status"; led-set = <0>; diff --git a/target/linux/realtek/dts/rtl9303_xikestor_sks8300-8x.dts b/target/linux/realtek/dts/rtl9303_xikestor_sks8300-8x.dts index 87131006964..b3b2a9a6f1b 100644 --- a/target/linux/realtek/dts/rtl9303_xikestor_sks8300-8x.dts +++ b/target/linux/realtek/dts/rtl9303_xikestor_sks8300-8x.dts @@ -304,8 +304,8 @@ port@0 { reg = <0>; label = "lan1"; - pseudo-phy-handle = <&phy0>; - phy-mode = "10gbase-r"; + phy-handle = <&phy0>; + phy-mode = "1000base-x"; sfp = <&sfp0>; managed = "in-band-status"; led-set = <0>; @@ -314,8 +314,8 @@ port@8 { reg = <8>; label = "lan2"; - pseudo-phy-handle = <&phy8>; - phy-mode = "10gbase-r"; + phy-handle = <&phy8>; + phy-mode = "1000base-x"; sfp = <&sfp1>; managed = "in-band-status"; led-set = <0>; @@ -324,8 +324,8 @@ port@10 { reg = <16>; label = "lan3"; - pseudo-phy-handle = <&phy16>; - phy-mode = "10gbase-r"; + phy-handle = <&phy16>; + phy-mode = "1000base-x"; sfp = <&sfp2>; managed = "in-band-status"; led-set = <0>; @@ -334,8 +334,8 @@ port@14 { reg = <20>; label = "lan4"; - pseudo-phy-handle = <&phy20>; - phy-mode = "10gbase-r"; + phy-handle = <&phy20>; + phy-mode = "1000base-x"; sfp = <&sfp3>; managed = "in-band-status"; led-set = <0>; @@ -344,8 +344,8 @@ port@18 { reg = <24>; label = "lan5"; - pseudo-phy-handle = <&phy24>; - phy-mode = "10gbase-r"; + phy-handle = <&phy24>; + phy-mode = "1000base-x"; sfp = <&sfp4>; managed = "in-band-status"; led-set = <0>; @@ -354,8 +354,8 @@ port@19 { reg = <25>; label = "lan6"; - pseudo-phy-handle = <&phy25>; - phy-mode = "10gbase-r"; + phy-handle = <&phy25>; + phy-mode = "1000base-x"; sfp = <&sfp5>; managed = "in-band-status"; led-set = <0>; @@ -364,8 +364,8 @@ port@1a { reg = <26>; label = "lan7"; - pseudo-phy-handle = <&phy26>; - phy-mode = "10gbase-r"; + phy-handle = <&phy26>; + phy-mode = "1000base-x"; sfp = <&sfp6>; managed = "in-band-status"; led-set = <0>; @@ -374,8 +374,8 @@ port@1b { reg = <27>; label = "lan8"; - pseudo-phy-handle = <&phy27>; - phy-mode = "10gbase-r"; + phy-handle = <&phy27>; + phy-mode = "1000base-x"; sfp = <&sfp7>; managed = "in-band-status"; led-set = <0>; diff --git a/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/common.c b/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/common.c index f6bef5e90c4..fea0551ccaa 100644 --- a/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/common.c +++ b/target/linux/realtek/files-6.12/drivers/net/dsa/rtl83xx/common.c @@ -335,22 +335,6 @@ static int __init rtl83xx_mdio_probe(struct rtl838x_switch_priv *priv) continue; phy_node = of_parse_phandle(dn, "phy-handle", 0); - - /* Major cleanup is needed... - * - * We use virtual "phys" as containers for mac - * properties like the SERDES channel, even for simple - * SFP slots. "pseudo-phy-handle" is a hack to - * support this construct and still allow pluggable - * phys. - * - * The SERDES map is most likely static by port number - * for each SoC. No need to put that into the device - * tree in the first place. - */ - if (!phy_node) - phy_node = of_parse_phandle(dn, "pseudo-phy-handle", 0); - if (!phy_node) { if (pn != priv->cpu_port) dev_err(priv->dev, "Port node %d misses phy-handle\n", pn); -- 2.47.2