Re: [PATCH net v3] net: phy: mediatek-ge: disable EEE on the MT7530 PHY

Both findings confirmed against current sources -- thank you, the
analysis is spot on.

v4 keeps the probe()-time advertisement clear, which also feeds an
empty advertisement back into genphy_c45_read_eee_adv() during
phy_probe(), and moves the durable disable to config_init(), the
dp83867 placement: config_init() runs after of_set_phy_eee_broken() in
phy_probe(), so the eee_disabled_modes fill is not undone, and it is
the callback phy_init_hw() replays after a PHY reset, so the register
clear is restored as well. The register is also written again there
for the reset path.

v4 follows as a fresh thread.
https://lore.kernel.org/netdev/20260908145213.3976508-1-vladislav.karmanov.dev@gmail.com/ (local)

Tested on the affected hardware (ASUS RT-AX53U, MT7621): single clean
100 Mbps WAN link + DHCP lease over a 2-pair cable, no flapping. The
ethtool re-enable path is not reachable on this board (phylink rejects
EEE ops at the MAC layer), so that part is verified at the code level
for standalone use of the PHY.

вт, 8 сент. 2026 г. в 11:30, [off-list ref]:

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

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

- [High] mt7530_phy_probe() (drivers/net/phy/mediatek/mtk-ge.c:65-84)
  calls phy_disable_eee() from the driver's .probe callback, but phylib…
- [Medium] The hardware write that clears the EEE advertisement,
  phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0) at…

--

net: phy: mediatek-ge: disable EEE on the MT7530 PHY

This adds an mt7530_phy_probe() callback to the mediatek-ge driver which
clears MDIO_AN_EEE_ADV in hardware and then calls phy_disable_eee(), with
the stated goal that neither phylib nor userspace can re-enable EEE on the
MT7530 internal GE PHY.

quoted

diff --git a/drivers/net/phy/mediatek/mtk-ge.c b/drivers/net/phy/mediatek/mtk-ge.c
index 73d9b72f9d9e2..35600c70cab90 100644
--- a/drivers/net/phy/mediatek/mtk-ge.c
+++ b/drivers/net/phy/mediatek/mtk-ge.c

@@ -62,6 +62,27 @@ static void mtk_gephy_config_init(struct phy_device *phydev)
                     FIELD_PREP(MTK_MCC_NEARECHO_OFFSET_MASK, 0x3));
 }

+static int mt7530_phy_probe(struct phy_device *phydev)
+{
+     int ret;
+
+     /* The MT7530 internal GE PHY has broken EEE: with EEE advertised,
+      * some link partners fail to establish a stable link (on a 2-pair
+      * cable, 1000BASE-T training fails and the port loops instead of
+      * falling back). MediaTek recommends disabling EEE on this PHY.
+      * Clear the EEE advertisement early and mark EEE disabled, so
+      * that neither phylib nor userspace can re-enable it.
+      */
+
+     ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+     if (ret)
+             return ret;

[Severity: Medium]
Should this register write also live in mt7530_phy_config_init()?  This is
the only place where MDIO_AN_EEE_ADV is cleared in hardware, and ->probe is
the one callback phylib never replays.  phy_init_hw() deasserts the reset
line, optionally soft-resets, and then re-runs only ->config_init and
->config_intr:

drivers/net/phy/phy_device.c:phy_init_hw() {
        ...
        if (phydev->drv->config_init) {
                ret = phydev->drv->config_init(phydev);
                if (ret < 0)
                        return ret;
        }
        ...
}

mt7530_phy_config_init() does not touch MDIO_AN_EEE_ADV, so after a PHY
register reset the register is back at its EEE-advertising hardware default
while phydev->advertising_eee stays empty and eee_cfg.eee_enabled stays
false, i.e. software and hardware state disagree.

Two paths that re-init without ->probe:

  - phy_detach() asserts the PHY reset signal and a later
    phy_attach_direct() -> phy_init_hw() only replays config_init.
  - the SIOCSMIIREG BMCR_RESET case in phy_mii_ioctl():

drivers/net/phy/phy.c:phy_mii_ioctl() {
        ...
                if (prtad == phydev->mdio.addr &&
                    devad == MII_BMCR &&
                    val & BMCR_RESET)
                        return phy_init_hw(phydev);
        ...
}

which returns without any aneg reconfiguration, so nothing rewrites the
advertisement to zero.  __genphy_config_aneg() ->
genphy_c45_an_config_eee_aneg() covers the usual renegotiation case, but it
returns early without writing when phydev->autoneg == AUTONEG_DISABLE.  The
equivalent in-tree quirk issues the disable from the replayed callback,
dp83867_config_init() -> phy_disable_eee().

quoted

+
+     phy_disable_eee(phydev);

[Severity: High]
Does the phy_disable_eee() state survive the rest of phy_probe()?  The
linkmode_fill(phydev->eee_disabled_modes) done inside phy_disable_eee() is
what makes the disable permanent, but phylib clears that set later in the
same probe flow:

drivers/net/phy/phy_device.c:phy_probe() {
        ...
        if (phydev->drv->probe) {
                err = phydev->drv->probe(phydev);
        ...
        /* Get the EEE modes we want to prohibit. */
        of_set_phy_eee_broken(phydev);
        ...
}

drivers/net/phy/phy-core.c:of_set_phy_eee_broken() {
        struct device_node *node = phydev->mdio.dev.of_node;
        unsigned long *modes = phydev->eee_disabled_modes;

        if (!IS_ENABLED(CONFIG_OF_MDIO) || !node)
                return;

        linkmode_zero(modes);
        ...
}

So for any MT7530 PHY that has an of_node, eee_disabled_modes ends up empty
again unless the DT carries eee-broken-* properties.  An of_node is present
in a documented configuration: mt7530_setup_mdio() registers the internal
bus with the switch's "mdio" child node,

drivers/net/dsa/mt7530.c:mt7530_setup_mdio() {
        ...
        mnp = of_get_child_by_name(np, "mdio");
        ...
        ret = devm_of_mdiobus_register(dev, bus, mnp);
        ...
}

and mediatek,mt7530.yaml documents that node, so ethernet-phy children
described there get an of_node.

With eee_disabled_modes empty, can userspace re-enable EEE again through
dsa_user_set_eee() -> phylink_ethtool_set_eee() -> phy_ethtool_set_eee() ->
genphy_c45_ethtool_set_eee()?

drivers/net/phy/phy-c45.c:genphy_c45_ethtool_set_eee() {
        ...
                } else if (linkmode_empty(phydev->advertising_eee)) {
                        phy_advertise_eee_all(phydev);
                }
        ...
}

That repopulates advertising_eee with supported_eee, then
genphy_c45_an_config_eee_aneg() rewrites MDIO_AN_EEE_ADV and
phy_restart_aneg() renegotiates, which looks like it brings back exactly the
1000BASE-T training loop described in the commit message.

Would moving the disable into mt7530_phy_config_init(), which runs after
of_set_phy_eee_broken(), address both this and the reset case above?  That
is the placement dp83867_config_init() uses for the same kind of quirk.

[ ... ]

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