[PATCH net-next v2 0/5] net: phy: Disable MDIO broadcast address on YT8821

[PATCH net-next v2 0/5] net: phy: Disable MDIO broadcast address on YT8821

[Jakub Vaněk]

Hello,

this series is a rewrite of patch [1], which attempted to make the
Motorcomm YT8821 PHY operate reliably in the Cudy M3000 WiFi router.

Background
==========

The issue on the Cudy M3000 is an MDIO address collision at address 0:

 * The MediaTek MT7981B internal Gigabit PHY appears to be hardwired
   to respond only at MDIO address 0.

 * The Motorcomm YT8821 external PHY responds, by default, at address 0
   in addition to address 1 selected by its strapping pins.

At a minimum, this means that MDIO transactions intended for the
MT7981B PHY are also interpreted by the YT8821, which causes the
YT8821 to not work reliably.

The YT8821 is not unique in this regard. At least two other vendors
ship PHYs with similar behavior:

 * Realtek RTL8221B-VB-CG
 * Micrel KSZ8081

It appears to me that multiple vendors may have interpreted IEEE 802.3
Clause 22.2.4.5.5 to mean that MDIO address 0 is a reserved broadcast
address ("A PHY [...] shall always respond to transactions addressed
to PHY Address zero"). However, the omitted part of that sentence
limits the scope, and it does not apply to many PHY types.

What this series does
=====================

The goal of this series is to reconfigure the YT8821 early enough so
that it stops responding on address 0 before the kernel performs the
first access to MDIO address 0.

The approach is as follows:

 1. Patches 1 and 2 modify the MDIO bus probing order so that address 0
    is scanned last. This allows PHY fixups to reconfigure PHYs found
    at addresses 1-31 before address 0 is probed. The core idea was
    suggested by @dangowrt on OpenWrt GitHub [2].

 2. Patches 3-5 implement the required PHY fixup for the YT8821.
    They introduce infrastructure for an "address 0 fixup" that can
    be reused by other affected PHYs.

I initially attempted to use the driver .probe() callback. However,
testing showed that this is insufficient: if the YT8821 driver is
built as a module, the .probe() callback does not run early
enough (i.e. before the MT7981B PHY is initialized).

Why this is done in the kernel
==============================

In the v1 discussion [1], the conclusion was that this should ideally
be handled in the bootloader. However, this is not always practical
for OpenWrt deployments:

 * On the Cudy M3000, replacing the stock U-Boot is possible, but
   doing so removes the ability to easily revert to the vendor
   firmware using the procedure described in [3].

 * On other platforms, replacing the bootloader may not be possible, and
   so OpenWrt may need to have a kernel-based workaround for them [2].

Testing
=======

The series was tested against the v6.12 kernel currently used by
OpenWrt main. It resolves the PHY issues on the Cudy M3000 with the
Motorcomm PHY and does not break networking on the Cudy WR3000H,
which uses a different Realtek PHY.

I also verified that no MDIO access to address 0 occurs before the
YT8821 is reconfigured to stop responding on that address.

The series is rebased on top of v7.0-rc1 and build-tested there.

[1]: https://lore.kernel.org/all/d3bb9c36-5a0e-4339-901d-2dd21bdba395@gmail.com/#t
[2]: https://github.com/openwrt/openwrt/pull/21584#issuecomment-3941868545
[3]: https://www.cudy.com/en-eu/blogs/faq/how-to-recovery-the-cudy-router-from-openwrt-firmware-to-cudy-official-firmware

Signed-off-by: Jakub Vaněk <linuxtardis@gmail.com>
---
Changes in v2:
- Introduce changes into the PHY core that allow the broadcast
  addresses to be disabled before the first address collision
  occurs.
- Move the early disabling of the broadcast address from the
  YT8821 .probe() callback into a PHY fixup.
- In motorcomm.c, use ytphy_read_ext_with_lock() for the disabling
  as it takes the MDIO bus lock.

Jakub Vaněk (5):
  net: mdio_bus: Scan buses in reverse order (31 -> 0)
  of: mdio: Scan PHY address 0 last
  net: phy: Support PHY fixups on Clause 45 PHYs
  net: phy: Add infrastructure for PHY address 0 fixups
  net: phy: motorcomm: yt8821: Disable MDIO broadcast

 drivers/net/mdio/of_mdio.c          | 35 +++++++++++++++--
 drivers/net/phy/Makefile            |  3 +-
 drivers/net/phy/mdio_bus_provider.c | 14 ++++++-
 drivers/net/phy/motorcomm.c         | 29 ++++++++++++++
 drivers/net/phy/phy_common_fixups.c | 59 ++++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c        | 60 ++++++++++++++++++++---------
 drivers/net/phy/phylib-internal.h   |  2 +
 7 files changed, 176 insertions(+), 26 deletions(-)
 create mode 100644 drivers/net/phy/phy_common_fixups.c

-- 
2.43.0

[PATCH net-next v2 1/5] net: mdiobus: Scan buses in reverse order (31 -> 0)

[Jakub Vaněk]

Some PHY devices incorrectly treat address 0 as a broadcast address.
As a result, accesses to address 0 may cause multiple PHYs to respond,
making one or both PHYs work unreliably. In other cases, the PHY may be
detected twice by Linux: once at address 0 and once at its actual
address).

On several PHYs (e.g. Motorcomm YT8821 and Realtek RTL8221B), this
behavior can be disabled via a vendor-specific internal register.
However, for that to be useful, that register would have to be
programmed before address 0 is accessed for the first time.

On non-Device Tree systems, MDIO buses are typically scanned in
mdiobus_register(). Change the address scan order from 0->31 to 31->0
so that PHY fixups are applied to addresses 1-31 before address 0
is probed. This way the address collision can be avoided.

The change is implemented separately for Clause 22 and Clause 45 PHYs.
This approach might still leave some collisions at address 0 unhandled,
but it is much easier to implement.

Device Tree-based systems also require a different approach. In that case,
of_mdiobus_register() probes only the addresses explicitly described
in the Device Tree. Handling for DT-based systems is implemented in
a separate commit.

Suggested-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Jakub Vaněk <linuxtardis@gmail.com>
---
 drivers/net/phy/mdio_bus_provider.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mdio_bus_provider.c b/drivers/net/phy/mdio_bus_provider.c
index 4b0637405740..d3454d229795 100644
--- a/drivers/net/phy/mdio_bus_provider.c
+++ b/drivers/net/phy/mdio_bus_provider.c
@@ -208,7 +208,12 @@ static int mdiobus_scan_bus_c22(struct mii_bus *bus)
 {
 	int i;
 
-	for (i = 0; i < PHY_MAX_ADDR; i++) {
+	/* Scan address 0 last. Some vendors consider it a broadcast address
+	 * and so their PHYs respond at it in addition to the actual PHY address.
+	 * Scanning addresses 1-31 first allows PHY fixups to reconfigure these
+	 * PHYs to not respond at address 0 before we try to scan it.
+	 */
+	for (i = PHY_MAX_ADDR - 1; i >= 0; i--) {
 		if ((bus->phy_mask & BIT(i)) == 0) {
 			struct phy_device *phydev;
 
@@ -224,7 +229,12 @@ static int mdiobus_scan_bus_c45(struct mii_bus *bus)
 {
 	int i;
 
-	for (i = 0; i < PHY_MAX_ADDR; i++) {
+	/* Scan address 0 last. Some vendors consider it a broadcast address
+	 * and so their PHYs respond at it in addition to the actual PHY address.
+	 * Scanning addresses 1-31 first allows PHY fixups to reconfigure these
+	 * PHYs to not respond at address 0 before we try to scan it.
+	 */
+	for (i = PHY_MAX_ADDR - 1; i >= 0; i--) {
 		if ((bus->phy_mask & BIT(i)) == 0) {
 			struct phy_device *phydev;
 
-- 
2.43.0

[PATCH net-next v2 2/5] of: mdio: Scan PHY address 0 last

[Jakub Vaněk]

Some PHY devices incorrectly treat address 0 as a broadcast address.
As a result, accesses to address 0 may cause multiple PHYs to respond,
making one or both PHYs work unreliably.

On several PHYs (e.g. Motorcomm YT8821 and Realtek RTL8221B), this
behavior can be disabled via a vendor-specific internal register.
However, for that to be useful, that register would have to be
programmed before address 0 is accessed for the first time.

Device Tree-based systems scan MDIO buses via of_mdiobus_register().
Modify the scanning order so that address 0 is scanned last. This
ensures PHY fixups for addresses 1-31 are applied before address 0
is accessed, allowing the collision to be prevented.

However, preserve the original probing order for one edge case: when
the Device Tree does not explicitly specify PHY addresses. In that
scenario, PHY DT nodes appear to be matched to devices based on a
sequential bus scan. Changing the scanning sequence could change
the association between DT nodes and PHY devices and potentially break
existing setups.

For example, with:

    mdio-bus {
        phy0: ethernet-phy {
            compatible = "ethernet-phy-id1234.5678";
        };
        phy1: ethernet-phy {
            compatible = "ethernet-phy-id90AB.CDEF";
        };
    };

scanning address 0 last could cause the PHY on address 0 to be associated
with the phy1 node.

Suggested-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Jakub Vaněk <linuxtardis@gmail.com>
---
 drivers/net/mdio/of_mdio.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index b8d298c04d3f..a705991f6b04 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -27,6 +27,11 @@ MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("OpenFirmware MDIO bus (Ethernet PHY) accessors");
 
+enum scan_phase {
+	SCAN_PHASE_NOT_PHYAD_0,
+	SCAN_PHASE_PHYAD_0,
+};
+
 /* Extract the clause 22 phy ID from the compatible string of the form
  * ethernet-phy-idAAAA.BBBB */
 static int of_get_phy_id(struct device_node *device, u32 *phy_id)
@@ -137,7 +142,7 @@ bool of_mdiobus_child_is_phy(struct device_node *child)
 EXPORT_SYMBOL(of_mdiobus_child_is_phy);
 
 static int __of_mdiobus_parse_phys(struct mii_bus *mdio, struct device_node *np,
-				   bool *scanphys)
+				   bool *scanphys, enum scan_phase phase)
 {
 	struct device_node *child;
 	int addr, rc = 0;
@@ -149,7 +154,7 @@ static int __of_mdiobus_parse_phys(struct mii_bus *mdio, struct device_node *np,
 			if (!of_property_present(child, "reg"))
 				continue;
 
-			rc = __of_mdiobus_parse_phys(mdio, child, NULL);
+			rc = __of_mdiobus_parse_phys(mdio, child, NULL, phase);
 			if (rc && rc != -ENODEV)
 				goto exit;
 
@@ -164,6 +169,12 @@ static int __of_mdiobus_parse_phys(struct mii_bus *mdio, struct device_node *np,
 			continue;
 		}
 
+		if (phase == SCAN_PHASE_NOT_PHYAD_0 && addr == 0)
+			continue;
+
+		if (phase == SCAN_PHASE_PHYAD_0 && addr != 0)
+			continue;
+
 		if (of_mdiobus_child_is_phy(child))
 			rc = of_mdiobus_register_phy(mdio, child, addr);
 		else
@@ -223,8 +234,19 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np,
 	if (rc)
 		return rc;
 
-	/* Loop over the child nodes and register a phy_device for each phy */
-	rc = __of_mdiobus_parse_phys(mdio, np, &scanphys);
+	/* Loop over the child nodes and register a phy_device for each phy.
+	 * However, scan address 0 last. Some vendors consider it a broadcast
+	 * address and so their PHYs respond at it in addition to the actual PHY
+	 * address. Scanning addresses 1-31 first allows PHY fixups to stop
+	 * the potential collision at address 0 from occurring.
+	 */
+	rc = __of_mdiobus_parse_phys(mdio, np, &scanphys,
+				     SCAN_PHASE_NOT_PHYAD_0);
+	if (rc)
+		goto unregister;
+
+	rc = __of_mdiobus_parse_phys(mdio, np, &scanphys,
+				     SCAN_PHASE_PHYAD_0);
 	if (rc)
 		goto unregister;
 
@@ -238,6 +260,11 @@ int __of_mdiobus_register(struct mii_bus *mdio, struct device_node *np,
 		    of_node_name_eq(child, "ethernet-phy-package"))
 			continue;
 
+		/* Skip the SCAN_PHASE_NOT_PHYAD_0/SCAN_PHASE_PHYAD_0
+		 * stuff here. Some device tree setups may assume linear
+		 * assignment from address 0 onwards and the two-pass probing
+		 * is not worth breaking these setups.
+		 */
 		for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
 			/* skip already registered PHYs */
 			if (mdiobus_is_registered_device(mdio, addr))
-- 
2.43.0

[PATCH net-next v2 3/5] net: phy: Support PHY fixups on Clause 45 PHYs

[Jakub Vaněk]

For Clause 45 PHYs, phydev->phy_id is zero and therefore cannot be used
to match PHY fixups. Clause 45 PHYs instead have per-MMD identifiers
stored in phydev->c45_ids. Extend phy_needs_fixup() to match against
these IDs by reusing the relevant matching logic from
genphy_match_phy_device().

Signed-off-by: Jakub Vaněk <linuxtardis@gmail.com>
---
 drivers/net/phy/phy_device.c | 56 ++++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 19 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 02fc0133428d..db187d370eaa 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -472,13 +472,48 @@ int phy_register_fixup_for_id(const char *bus_id,
 }
 EXPORT_SYMBOL(phy_register_fixup_for_id);
 
+/**
+ * genphy_check_device_id - match a PHY device against a given ID
+ * @phydev: target phy_device struct
+ * @phy_id: expected PHY ID
+ * @phy_id_mask: the PHY ID mask, set bits are significant in matching
+ *
+ * Description: Checks whether the given PHY device matches the specified
+ * ID. For Clause 45 PHYs, iterates over the available device identifiers
+ * and compares them against the expected PHY ID, applying the provided mask.
+ * For Clause 22 PHYs, a direct ID comparison is performed.
+ *
+ * Return: true if the PHY device matches the masked ID, false otherwise.
+ */
+static bool genphy_check_device_id(struct phy_device *phydev,
+				   u32 phy_id, u32 phy_id_mask)
+{
+	if (phydev->is_c45) {
+		const int num_ids = ARRAY_SIZE(phydev->c45_ids.device_ids);
+		int i;
+
+		for (i = 1; i < num_ids; i++) {
+			if (phydev->c45_ids.device_ids[i] == 0xffffffff)
+				continue;
+
+			if (phy_id_compare(phydev->c45_ids.device_ids[i],
+					   phy_id, phy_id_mask))
+				return true;
+		}
+
+		return false;
+	}
+
+	return phy_id_compare(phydev->phy_id, phy_id, phy_id_mask);
+}
+
 static bool phy_needs_fixup(struct phy_device *phydev, struct phy_fixup *fixup)
 {
 	if (!strcmp(fixup->bus_id, phydev_name(phydev)))
 		return true;
 
 	if (fixup->phy_uid_mask &&
-	    phy_id_compare(phydev->phy_id, fixup->phy_uid, fixup->phy_uid_mask))
+	    genphy_check_device_id(phydev, fixup->phy_uid, fixup->phy_uid_mask))
 		return true;
 
 	return false;
@@ -522,24 +557,7 @@ static int phy_scan_fixups(struct phy_device *phydev)
 int genphy_match_phy_device(struct phy_device *phydev,
 			    const struct phy_driver *phydrv)
 {
-	if (phydev->is_c45) {
-		const int num_ids = ARRAY_SIZE(phydev->c45_ids.device_ids);
-		int i;
-
-		for (i = 1; i < num_ids; i++) {
-			if (phydev->c45_ids.device_ids[i] == 0xffffffff)
-				continue;
-
-			if (phy_id_compare(phydev->c45_ids.device_ids[i],
-					   phydrv->phy_id, phydrv->phy_id_mask))
-				return 1;
-		}
-
-		return 0;
-	}
-
-	return phy_id_compare(phydev->phy_id, phydrv->phy_id,
-			      phydrv->phy_id_mask);
+	return genphy_check_device_id(phydev, phydrv->phy_id, phydrv->phy_id_mask);
 }
 EXPORT_SYMBOL_GPL(genphy_match_phy_device);
 
-- 
2.43.0

[PATCH net-next v2 4/5] net: phy: Add infrastructure for PHY address 0 fixups

[Jakub Vaněk]

Certain PHYs incorrectly treat MDIO address 0 as a broadcast address and
therefore respond both at address 0 and at their assigned address.
As outlined in the code, this is problematic. Provide a way to
disable this behaviour via a PHY fixup.

It would be ideal to have these collisions prevented early
by the bootloader, but there may be hardware (e.g. WiFi routers)
out there where the bootloader doesn't do this and cannot be easily
replaced.

Signed-off-by: Jakub Vaněk <linuxtardis@gmail.com>
---
 drivers/net/phy/Makefile            |  3 ++-
 drivers/net/phy/phy_common_fixups.c | 26 ++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c        |  4 ++++
 drivers/net/phy/phylib-internal.h   |  2 ++
 4 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/phy/phy_common_fixups.c

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 3a34917adea7..3bf9ba208b90 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -3,7 +3,8 @@
 
 libphy-y			:= phy.o phy-c45.o phy-core.o phy_device.o \
 				   linkmode.o phy_link_topology.o \
-				   phy_caps.o mdio_bus_provider.o phy_port.o
+				   phy_caps.o mdio_bus_provider.o phy_port.o \
+				   phy_common_fixups.o
 mdio-bus-y			+= mdio_bus.o mdio_device.o
 
 ifdef CONFIG_PHYLIB
diff --git a/drivers/net/phy/phy_common_fixups.c b/drivers/net/phy/phy_common_fixups.c
new file mode 100644
index 000000000000..fa51c6d7b25f
--- /dev/null
+++ b/drivers/net/phy/phy_common_fixups.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/phy.h>
+
+#include "phylib-internal.h"
+
+/**
+ * phy_register_address_0_fixups - Register fixups for disabling MDIO
+ * broadcast address 0
+ *
+ * Some vendors interpret MDIO address 0 as a broadcast address and so their
+ * PHYs initially respond both at address 0 as well as at their normal
+ * address. This is problematic:
+ *  - Linux may create two struct phy_device for a single device.
+ *  - MDIO address collision may occur if there is another PHY
+ *    legitimately listening on address 0.
+ *
+ * The broadcast address can often be disabled through some internal
+ * PHY register. These fixups are designed to do just that --
+ * they shall stop these PHYs from responding at address 0 before
+ * the MDIO address 0 is scanned for devices.
+ */
+int phy_register_address_0_fixups(void)
+{
+	return 0;
+}
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index db187d370eaa..2d4f119b45b4 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -3969,6 +3969,10 @@ static int __init phy_init(void)
 	if (rc)
 		goto err_ethtool_phy_ops;
 
+	rc = phy_register_address_0_fixups();
+	if (rc)
+		goto err_ethtool_phy_ops;
+
 	features_init();
 
 	rc = phy_driver_register(&genphy_c45_driver, THIS_MODULE);
diff --git a/drivers/net/phy/phylib-internal.h b/drivers/net/phy/phylib-internal.h
index dc9592c6bb8e..1e6ff8f2d721 100644
--- a/drivers/net/phy/phylib-internal.h
+++ b/drivers/net/phy/phylib-internal.h
@@ -22,4 +22,6 @@ void phy_check_downshift(struct phy_device *phydev);
 
 int genphy_c45_read_eee_adv(struct phy_device *phydev, unsigned long *adv);
 
+int phy_register_address_0_fixups(void);
+
 #endif /* __PHYLIB_INTERNAL_H */
-- 
2.43.0

[PATCH net-next v2 5/5] net: phy: motorcomm: yt8821: Disable MDIO broadcast

[Jakub Vaněk]

By default, the YT8821 responds at two MDIO addresses: the address
selected by its strapping pins and the broadcast address 0.
This causes problems if another PHY is legitimately configured
at address 0.

Disable the broadcast address using two mechanisms. The PHY fixup
prevents collisions during initial MDIO bus scanning.
yt8821_config_init() then re-disables the broadcast address if the
PHY goes through a hardware reset (this happens if the PHY has a
reset GPIO on its device tree node, and the interface is brought
down and up).

Link: https://github.com/openwrt/openwrt/pull/21584
Fixes: b671105b88c3 ("net: phy: Add driver for Motorcomm yt8821 2.5G ethernet phy")
Signed-off-by: Jakub Vaněk <linuxtardis@gmail.com>
---
 drivers/net/phy/motorcomm.c         | 29 +++++++++++++++++++++++++
 drivers/net/phy/phy_common_fixups.c | 33 +++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 4d62f7b36212..e1503fab568a 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -227,6 +227,9 @@
 #define YT8521_LED_100_ON_EN			BIT(5)
 #define YT8521_LED_10_ON_EN			BIT(4)
 
+#define YTPHY_MDIO_ADDRESS_CONTROL_REG		0xA005
+#define YTPHY_MACR_EN_PHY_ADDR_0		BIT(6)
+
 #define YTPHY_MISC_CONFIG_REG			0xA006
 #define YTPHY_MCR_FIBER_SPEED_MASK		BIT(0)
 #define YTPHY_MCR_FIBER_1000BX			(0x1 << 0)
@@ -2764,6 +2767,28 @@ static int yt8821_soft_reset(struct phy_device *phydev)
 					  YT8521_CCR_SW_RST, 0);
 }
 
+/**
+ * yt8821_disable_mdio_address_zero() - disable MDIO broadcast address 0
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * The YT8821 responds on two MDIO addresses by default:
+ *  - the address selected by its strapping pins
+ *  - the broadcast address 0
+ *
+ * Some other PHYs (e.g. the MT7981B internal Gigabit PHY) are hardwired to
+ * respond only at MDIO address 0. If the YT8821 also listens on address 0,
+ * it may incorrectly react to transactions intended for those PHYs.
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt8821_disable_mdio_address_zero(struct phy_device *phydev)
+{
+	return ytphy_modify_ext_with_lock(phydev,
+					  YTPHY_MDIO_ADDRESS_CONTROL_REG,
+					  YTPHY_MACR_EN_PHY_ADDR_0,
+					  0);
+}
+
 /**
  * yt8821_config_init() - phy initializatioin
  * @phydev: a pointer to a &struct phy_device
@@ -2799,6 +2824,10 @@ static int yt8821_config_init(struct phy_device *phydev)
 		phydev->rate_matching = RATE_MATCH_PAUSE;
 	}
 
+	ret = yt8821_disable_mdio_address_zero(phydev);
+	if (ret < 0)
+		return ret;
+
 	ret = yt8821_serdes_init(phydev);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/net/phy/phy_common_fixups.c b/drivers/net/phy/phy_common_fixups.c
index fa51c6d7b25f..5a60721b56d6 100644
--- a/drivers/net/phy/phy_common_fixups.c
+++ b/drivers/net/phy/phy_common_fixups.c
@@ -4,6 +4,32 @@
 
 #include "phylib-internal.h"
 
+#define PHY_ID_YT8821				0x4f51ea19
+#define YTPHY_PAGE_SELECT			0x1E
+#define YTPHY_PAGE_DATA				0x1F
+#define YTPHY_MDIO_ADDRESS_CONTROL_REG		0xA005
+#define YTPHY_MACR_EN_PHY_ADDR_0		BIT(6)
+
+/**
+ * yt8821_disable_broadcast - Disable MDIO broadcast on address 0
+ */
+static int yt8821_disable_broadcast(struct phy_device *phydev)
+{
+	int rc = 0;
+
+	phy_lock_mdio_bus(phydev);
+
+	rc = __phy_write(phydev, YTPHY_PAGE_SELECT, YTPHY_MDIO_ADDRESS_CONTROL_REG);
+	if (rc < 0)
+		goto unlock;
+
+	rc = __phy_modify(phydev, YTPHY_PAGE_DATA, YTPHY_MACR_EN_PHY_ADDR_0, 0);
+
+unlock:
+	phy_unlock_mdio_bus(phydev);
+	return rc;
+}
+
 /**
  * phy_register_address_0_fixups - Register fixups for disabling MDIO
  * broadcast address 0
@@ -22,5 +48,12 @@
  */
 int phy_register_address_0_fixups(void)
 {
+	int rc;
+
+	rc = phy_register_fixup_for_uid(PHY_ID_YT8821, 0xFFFFFFFF,
+					yt8821_disable_broadcast);
+	if (rc < 0)
+		return rc;
+
 	return 0;
 }
-- 
2.43.0

Re: [PATCH net-next v2 5/5] net: phy: motorcomm: yt8821: Disable MDIO broadcast

[kernel test robot]

Hi Jakub,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Jakub-Van-k/net-mdiobus-Scan-buses-in-reverse-order-31-0/20260301-072526
base:   net-next/main
patch link:    https://lore.kernel.org/r/20260228232241.1274236-6-linuxtardis%40gmail.com
patch subject: [PATCH net-next v2 5/5] net: phy: motorcomm: yt8821: Disable MDIO broadcast
config: nios2-defconfig (https://download.01.org/0day-ci/archive/20260301/202603011047.rzQIXPzJ-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260301/202603011047.rzQIXPzJ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603011047.rzQIXPzJ-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/net/phy/phy_common_fixups.c:16 function parameter 'phydev' not described in 'yt8821_disable_broadcast'
>> Warning: drivers/net/phy/phy_common_fixups.c:16 function parameter 'phydev' not described in 'yt8821_disable_broadcast'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v2 1/5] net: mdiobus: Scan buses in reverse order (31 -> 0)

[Andrew Lunn]

On Sun, Mar 01, 2026 at 12:22:37AM +0100, Jakub Vaněk wrote:
> Some PHY devices incorrectly treat address 0 as a broadcast address.
> As a result, accesses to address 0 may cause multiple PHYs to respond,
> making one or both PHYs work unreliably. In other cases, the PHY may be
> detected twice by Linux: once at address 0 and once at its actual
> address).
> 
> On several PHYs (e.g. Motorcomm YT8821 and Realtek RTL8221B), this
> behavior can be disabled via a vendor-specific internal register.
> However, for that to be useful, that register would have to be
> programmed before address 0 is accessed for the first time.
> 
> On non-Device Tree systems, MDIO buses are typically scanned in
> mdiobus_register(). Change the address scan order from 0->31 to 31->0
> so that PHY fixups are applied to addresses 1-31 before address 0
> is probed. This way the address collision can be avoided.

Please add a comment about phy_package_join() etc. You need to
indicate this change has not broken any assumptions. It might also be
good to extend the documentation about PHY packages.

     Andrew

Re: [PATCH net-next v2 0/5] net: phy: Disable MDIO broadcast address on YT8821

[Andrew Lunn]

On Sun, Mar 01, 2026 at 12:22:36AM +0100, Jakub Vaněk wrote:
> Hello,
> 
> this series is a rewrite of patch [1], which attempted to make the
> Motorcomm YT8821 PHY operate reliably in the Cudy M3000 WiFi router.
> 
> Background
> ==========
> 
> The issue on the Cudy M3000 is an MDIO address collision at address 0:
> 
>  * The MediaTek MT7981B internal Gigabit PHY appears to be hardwired
>    to respond only at MDIO address 0.
> 
>  * The Motorcomm YT8821 external PHY responds, by default, at address 0
>    in addition to address 1 selected by its strapping pins.
> 
> At a minimum, this means that MDIO transactions intended for the
> MT7981B PHY are also interpreted by the YT8821, which causes the
> YT8821 to not work reliably.
> 
> The YT8821 is not unique in this regard. At least two other vendors
> ship PHYs with similar behavior:
> 
>  * Realtek RTL8221B-VB-CG
>  * Micrel KSZ8081
> 
> It appears to me that multiple vendors may have interpreted IEEE 802.3
> Clause 22.2.4.5.5 to mean that MDIO address 0 is a reserved broadcast
> address ("A PHY [...] shall always respond to transactions addressed
> to PHY Address zero"). However, the omitted part of that sentence
> limits the scope, and it does not apply to many PHY types.

I stopped being lazy and looked at 802.3:

  22.2.4.5.5 PHYAD (PHY Address)

  The PHY Address is five bits, allowing 32 unique PHY addresses. The
  first PHY address bit transmitted and received is the MSB of the
  address. A PHY that is connected to the station management entity
  via the mechanical interface defined in 22.6 shall always respond to
  transactions addressed to PHY Address zero <00000>. A station
  management entity that is attached to multiple PHYs has to have
  prior knowledge of the appropriate PHY Address for each PHY.

And

  22.6 Mechanical characteristics

  When the MII is used to interconnect two printed circuit assemblies
  via a short length of cable, the cable shall be connected to the
  circuit assembly that implements the Reconciliation sublayer by
  means of the mechanical interface defined in this clause.

  22.6.1 Definition of mechanical interface

  A 40-pole connector having the mechanical mateability dimensions as
  specified in IEC 61076-3-101:1997 shall be used for the MII
  connector. The circuit assembly that contains the MAC sublayer and
  Reconciliation sublayer shall have a female connector with screw
  locks, and the mating cable shall have a male connector with jack
  screws.

Does your board have this 40-pole connector and screw locks?

I don't think i have seen one of these in the last 30 years. I do
remember during my University times some Sun Microsystems, maybe a
Sun-2, with a fat maybe 2 meter cable running into the false floor to
a transceiver box onto the one Ethernet cable which snaked around the
Computing Department.

So this should not apply to any board from this century which is
conforming to the 802.3 standard.

So i would say the board you are trying to support is broken twice.

1) It is using part of 802.3 which does not apply.

2) It has two PHYs which using the same address.

I can understand a PHY might support this, optionally, with a
pull-up/down strapping, saying "Break the standard, also respond to
address 0". But this clearly should be an opt in, and an extra
component is required.

I'm still not convinced Linux should be handling either of these
conditions. And even if it did, it should not be in the core, but
hidden away in a driver.

       Andrew

Re: [PATCH net-next v2 1/5] net: mdiobus: Scan buses in reverse order (31 -> 0)

[Russell King (Oracle)]

On Sun, Mar 01, 2026 at 12:22:37AM +0100, Jakub Vaněk wrote:
> Some PHY devices incorrectly treat address 0 as a broadcast address.
> As a result, accesses to address 0 may cause multiple PHYs to respond,
> making one or both PHYs work unreliably. In other cases, the PHY may be
> detected twice by Linux: once at address 0 and once at its actual
> address).
> 
> On several PHYs (e.g. Motorcomm YT8821 and Realtek RTL8221B), this
> behavior can be disabled via a vendor-specific internal register.
> However, for that to be useful, that register would have to be
> programmed before address 0 is accessed for the first time.
> 
> On non-Device Tree systems, MDIO buses are typically scanned in
> mdiobus_register(). Change the address scan order from 0->31 to 31->0
> so that PHY fixups are applied to addresses 1-31 before address 0
> is probed. This way the address collision can be avoided.

I've said no to this before.

The order of scanning may affect the order in which devices are added.
However, that doesn't really have that much bearing in the order in
which devices are bound to their drivers.

If the drivers are already registered, then as each device is
registered with the driver model, it will be offered to drivers. So
in this case, the order in which devices are proed will be the order
in which they are scanned.

However, if a driver is loaded after scanning is complete, then the
order in which devices are presented to the driver is not under the
control of phylib, but is down to the driver model code. The driver
model scans the bus_type's devices in some order, and attempts to
match each with the new driver.

If the driver is not present, then even if you scan in reverse order,
the "ghost" at address 0 will be found, because the driver hasn't had
the opportunity to reprogram the PHY to disable that.

In both cases, things get worse if -EPROBE_DEFER happens.

So, you can't definitively control the order in which devices are
probed, which means that anything that depends on that will be fragile.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 0/5] net: phy: Disable MDIO broadcast address on YT8821

[Russell King (Oracle)]

On Sun, Mar 01, 2026 at 05:06:58PM +0100, Andrew Lunn wrote:
> I can understand a PHY might support this, optionally, with a
> pull-up/down strapping, saying "Break the standard, also respond to
> address 0". But this clearly should be an opt in, and an extra
> component is required.
> 
> I'm still not convinced Linux should be handling either of these
> conditions.

+1.

> And even if it did, it should not be in the core, but
> hidden away in a driver.

It's not something a driver can handle, because by the time a driver
gets to see anything about the devices, we could've already scanned
the addresses and read bad IDs.

I believe we've said before that the board firmware should be fixing
this, not the kernel - I know I have said that on more than one
occasion about this broken hardware.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 0/5] net: phy: Disable MDIO broadcast address on YT8821

[Jakub Vaněk]

On 3/1/26 17:06, Andrew Lunn wrote:
> On Sun, Mar 01, 2026 at 12:22:36AM +0100, Jakub Vaněk wrote:
>> Hello,
>>
>> this series is a rewrite of patch [1], which attempted to make the
>> Motorcomm YT8821 PHY operate reliably in the Cudy M3000 WiFi router.
>>
>> Background
>> ==========
>>
>> The issue on the Cudy M3000 is an MDIO address collision at address 0:
>>
>>  * The MediaTek MT7981B internal Gigabit PHY appears to be hardwired
>>    to respond only at MDIO address 0.
>>
>>  * The Motorcomm YT8821 external PHY responds, by default, at address 0
>>    in addition to address 1 selected by its strapping pins.
>>
>> At a minimum, this means that MDIO transactions intended for the
>> MT7981B PHY are also interpreted by the YT8821, which causes the
>> YT8821 to not work reliably.
>>
>> The YT8821 is not unique in this regard. At least two other vendors
>> ship PHYs with similar behavior:
>>
>>  * Realtek RTL8221B-VB-CG
>>  * Micrel KSZ8081
>>
>> It appears to me that multiple vendors may have interpreted IEEE 802.3
>> Clause 22.2.4.5.5 to mean that MDIO address 0 is a reserved broadcast
>> address ("A PHY [...] shall always respond to transactions addressed
>> to PHY Address zero"). However, the omitted part of that sentence
>> limits the scope, and it does not apply to many PHY types.
> 
> I stopped being lazy and looked at 802.3:
> 
>   22.2.4.5.5 PHYAD (PHY Address)
> 
>   The PHY Address is five bits, allowing 32 unique PHY addresses. The
>   first PHY address bit transmitted and received is the MSB of the
>   address. A PHY that is connected to the station management entity
>   via the mechanical interface defined in 22.6 shall always respond to
>   transactions addressed to PHY Address zero <00000>. A station
>   management entity that is attached to multiple PHYs has to have
>   prior knowledge of the appropriate PHY Address for each PHY.
> 
> And
> 
>   22.6 Mechanical characteristics
> 
>   When the MII is used to interconnect two printed circuit assemblies
>   via a short length of cable, the cable shall be connected to the
>   circuit assembly that implements the Reconciliation sublayer by
>   means of the mechanical interface defined in this clause.
> 
>   22.6.1 Definition of mechanical interface
> 
>   A 40-pole connector having the mechanical mateability dimensions as
>   specified in IEC 61076-3-101:1997 shall be used for the MII
>   connector. The circuit assembly that contains the MAC sublayer and
>   Reconciliation sublayer shall have a female connector with screw
>   locks, and the mating cable shall have a male connector with jack
>   screws.
> 
> Does your board have this 40-pole connector and screw locks?
> 

No, it doesn't :D

> I don't think i have seen one of these in the last 30 years. I do
> remember during my University times some Sun Microsystems, maybe a
> Sun-2, with a fat maybe 2 meter cable running into the false floor to
> a transceiver box onto the one Ethernet cable which snaked around the
> Computing Department.
> 
> So this should not apply to any board from this century which is
> conforming to the 802.3 standard.
> 

I agree with that, this is what I was originally trying to say by
the omitted part limiting the scope, I just got the wording of the
last part of that sentence wrong.

The reason I think that some vendors must have misinterpreted the
specification is that they have claims in their datasheets like
(Micrel KSZ8081RNA datasheet [1]):

| PHY address 0h is defined as the broadcast PHY address according
| to the IEEE 802.3 Specification, and can be used to read/write to
| a single PHY device, or write to multiple PHY devices simultaneously

and so I don't necessarily think they intentionally wanted to
violate the spec (nor I think you implied that, though).

[1]: https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/KSZ8081RNA-RND-10BASE-T-100-BASE-TX-PHY-with-RMII-Support-DS00002199F.pdf

> So i would say the board you are trying to support is broken twice.
> 
> 1) It is using part of 802.3 which does not apply.
> 
> 2) It has two PHYs which using the same address.
> 
> I can understand a PHY might support this, optionally, with a
> pull-up/down strapping, saying "Break the standard, also respond to
> address 0". But this clearly should be an opt in, and an extra
> component is required.
> 

This appears to be the case for some PHYs. I found that e.g. Micrel
KSZ8081MNX is documented to have a dedicated B-CAST_OFF pin that
does something like that. But e.g. Micrel KSZ8081RNA is shipped in
a smaller package that lacks that pin and its default is to use
address 0 as a broadcast address.

> I'm still not convinced Linux should be handling either of these
> conditions. And even if it did, it should not be in the core, but
> hidden away in a driver.
> 

ACK. Unfortunately I don't think it is feasible to move this into
the driver because of the probing order issues.

>        Andrew

Jakub

Re: [PATCH net-next v2 1/5] net: mdiobus: Scan buses in reverse order (31 -> 0)

[Jakub Vaněk]

On 3/1/26 18:03, Russell King (Oracle) wrote:
> On Sun, Mar 01, 2026 at 12:22:37AM +0100, Jakub Vaněk wrote:
>> Some PHY devices incorrectly treat address 0 as a broadcast address.
>> As a result, accesses to address 0 may cause multiple PHYs to respond,
>> making one or both PHYs work unreliably. In other cases, the PHY may be
>> detected twice by Linux: once at address 0 and once at its actual
>> address).
>>
>> On several PHYs (e.g. Motorcomm YT8821 and Realtek RTL8221B), this
>> behavior can be disabled via a vendor-specific internal register.
>> However, for that to be useful, that register would have to be
>> programmed before address 0 is accessed for the first time.
>>
>> On non-Device Tree systems, MDIO buses are typically scanned in
>> mdiobus_register(). Change the address scan order from 0->31 to 31->0
>> so that PHY fixups are applied to addresses 1-31 before address 0
>> is probed. This way the address collision can be avoided.
> 
> I've said no to this before.
> 
> The order of scanning may affect the order in which devices are added.
> However, that doesn't really have that much bearing in the order in
> which devices are bound to their drivers.
> 
> If the drivers are already registered, then as each device is
> registered with the driver model, it will be offered to drivers. So
> in this case, the order in which devices are proed will be the order
> in which they are scanned.
> 
> However, if a driver is loaded after scanning is complete, then the
> order in which devices are presented to the driver is not under the
> control of phylib, but is down to the driver model code. The driver
> model scans the bus_type's devices in some order, and attempts to
> match each with the new driver.
> 
> If the driver is not present, then even if you scan in reverse order,
> the "ghost" at address 0 will be found, because the driver hasn't had
> the opportunity to reprogram the PHY to disable that.
> 
> In both cases, things get worse if -EPROBE_DEFER happens.
> 
> So, you can't definitively control the order in which devices are
> probed, which means that anything that depends on that will be fragile.
> 

I have observed something similar -- when the YT8821 driver was
built as a module, the .probe callback was indeed not called early
enough. This is why I rewrote the patch to use a PHY fixup --
the fixups are called synchronously from phy_device_register() and
don't depend on the driver in any way. I thus agree that it is
not feasible to handle this inside the YT8821 kernel driver.

Jakub