mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v3] net: phy: mediatek-ge: disable EEE on the MT7530 PHY
@ 2026-09-04 20:28 Vladislav Karmanov
  2026-09-04 20:34 ` Andrew Lunn
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Vladislav Karmanov @ 2026-09-04 20:28 UTC (permalink / raw)
  To: netdev
  Cc: andrew, daniel, dqfext, SkyLake.Huang, hkallweit1, linux, davem,
	edumazet, kuba, pabeni, matthias.bgg, angelogioacchino.delregno,
	chester.a.unal, sean.wang, olteanv, yangshiji66, linux-kernel,
	linux-arm-kernel, linux-mediatek, Vladislav Karmanov

The MT7530 internal GE PHY advertises EEE by hardware default, but its
EEE support is defective: with EEE advertised, some link partners fail
to establish a stable link. On a 2-pair (4-wire) cable where both ends
advertise gigabit, 1000BASE-T training cannot succeed, and instead of
falling back to 100 Mbps the port loops, so no link or DHCP lease is
ever obtained. MediaTek confirms the hardware is the root cause (Landen
Chao, 2021): "EEE of the 10-year-old MT7530 internal gephy has many IOT
problems, so it is recommended to disable its EEE."

mtk_gephy_config_init() used to clear the EEE advertisement early, but
commit af3b4b0e59de ("net: phy: mediatek-ge: do not disable EEE
advertisement") removed that on the rationale that the DSA subdriver
already performs an early disable. That holds for MT7531, whose
mt7531_setup() clears MDIO_AN_EEE_ADV on each switch PHY, but not for
the MT7530 PHY: neither the MT7621 integrated switch nor the dedicated
MT7530 IC ever had such a loop, so removing it left those boards
without any working early EEE disable and the link flapping came back.

Since the broken hardware is the PHY, fix it in the PHY driver so it
covers all users of this PHY, integrated in a switch or standalone:

  - clear MDIO_AN_EEE_ADV in probe(), before anything can negotiate
    EEE with the link partner;
  - call phy_disable_eee() so phylib does not write the advertisement
    back on later renegotiations and userspace cannot re-enable EEE.

Auto-negotiation then falls back to a stable 100 Mbps link instead of
looping at gigabit. Tested on ASUS RT-AX53U (MT7621): with a 2-pair
cable on the WAN port, a single clean 100 Mbps link comes up and a
DHCP lease is obtained, where the unpatched driver loops.

Fixes: af3b4b0e59de ("net: phy: mediatek-ge: do not disable EEE advertisement")
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Vladislav Karmanov <vladislav.karmanov.dev@gmail.com>
---
Changes in v3:
- Move the fix from the DSA driver to the MT7530 PHY driver: the
  broken hardware is the PHY, so the workaround belongs there and
  covers all users of the PHY, switch-integrated or standalone
  (Andrew Lunn).
- Clear MDIO_AN_EEE_ADV from probe() rather than config_init() so the
  advertisement is off before anything can negotiate EEE, and call
  phy_disable_eee() so neither phylib nor userspace can re-enable it
  (Andrew Lunn).
- Remove the eee-broken-* device tree properties discussion from the
  commit message; the properties are not needed once the PHY driver
  disables broken EEE itself (Andrew Lunn).

v1: https://lore.kernel.org/netdev/20260818182829.1580811-1-vladislav.karmanov.dev@gmail.com/
v2: https://lore.kernel.org/netdev/20260820202844.1821687-1-vladislav.karmanov.dev@gmail.com/

 drivers/net/phy/mediatek/mtk-ge.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/phy/mediatek/mtk-ge.c b/drivers/net/phy/mediatek/mtk-ge.c
index 73d9b72..35600c7 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;
+
+	phy_disable_eee(phydev);
+
+	return 0;
+}
+
 static int mt7530_phy_config_init(struct phy_device *phydev)
 {
 	mtk_gephy_config_init(phydev);
@@ -100,6 +121,7 @@ static struct phy_driver mtk_gephy_driver[] = {
 	{
 		PHY_ID_MATCH_EXACT(MTK_GPHY_ID_MT7530),
 		.name		= "MediaTek MT7530 PHY",
+		.probe		= mt7530_phy_probe,
 		.config_init	= mt7530_phy_config_init,
 		/* Interrupts are handled by the switch, not the PHY
 		 * itself.

-- 
2.43.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v3] net: phy: mediatek-ge: disable EEE on the MT7530 PHY
  2026-09-04 20:28 [PATCH net v3] net: phy: mediatek-ge: disable EEE on the MT7530 PHY Vladislav Karmanov
@ 2026-09-04 20:34 ` Andrew Lunn
  2026-09-07  6:16 ` Qingfang Deng
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2026-09-04 20:34 UTC (permalink / raw)
  To: Vladislav Karmanov
  Cc: netdev, daniel, dqfext, SkyLake.Huang, hkallweit1, linux, davem,
	edumazet, kuba, pabeni, matthias.bgg, angelogioacchino.delregno,
	chester.a.unal, sean.wang, olteanv, yangshiji66, linux-kernel,
	linux-arm-kernel, linux-mediatek

On Fri, Sep 04, 2026 at 11:28:00PM +0300, Vladislav Karmanov wrote:
> The MT7530 internal GE PHY advertises EEE by hardware default, but its
> EEE support is defective: with EEE advertised, some link partners fail
> to establish a stable link. On a 2-pair (4-wire) cable where both ends
> advertise gigabit, 1000BASE-T training cannot succeed, and instead of
> falling back to 100 Mbps the port loops, so no link or DHCP lease is
> ever obtained. MediaTek confirms the hardware is the root cause (Landen
> Chao, 2021): "EEE of the 10-year-old MT7530 internal gephy has many IOT
> problems, so it is recommended to disable its EEE."
> 
> mtk_gephy_config_init() used to clear the EEE advertisement early, but
> commit af3b4b0e59de ("net: phy: mediatek-ge: do not disable EEE
> advertisement") removed that on the rationale that the DSA subdriver
> already performs an early disable. That holds for MT7531, whose
> mt7531_setup() clears MDIO_AN_EEE_ADV on each switch PHY, but not for
> the MT7530 PHY: neither the MT7621 integrated switch nor the dedicated
> MT7530 IC ever had such a loop, so removing it left those boards
> without any working early EEE disable and the link flapping came back.
> 
> Since the broken hardware is the PHY, fix it in the PHY driver so it
> covers all users of this PHY, integrated in a switch or standalone:
> 
>   - clear MDIO_AN_EEE_ADV in probe(), before anything can negotiate
>     EEE with the link partner;
>   - call phy_disable_eee() so phylib does not write the advertisement
>     back on later renegotiations and userspace cannot re-enable EEE.
> 
> Auto-negotiation then falls back to a stable 100 Mbps link instead of
> looping at gigabit. Tested on ASUS RT-AX53U (MT7621): with a 2-pair
> cable on the WAN port, a single clean 100 Mbps link comes up and a
> DHCP lease is obtained, where the unpatched driver loops.
> 
> Fixes: af3b4b0e59de ("net: phy: mediatek-ge: do not disable EEE advertisement")
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Vladislav Karmanov <vladislav.karmanov.dev@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v3] net: phy: mediatek-ge: disable EEE on the MT7530 PHY
  2026-09-04 20:28 [PATCH net v3] net: phy: mediatek-ge: disable EEE on the MT7530 PHY Vladislav Karmanov
  2026-09-04 20:34 ` Andrew Lunn
@ 2026-09-07  6:16 ` Qingfang Deng
  2026-09-08  8:30 ` netdev-bot+sashiko
  2026-10-12 16:35 ` Bjørn Mork
  3 siblings, 0 replies; 6+ messages in thread
From: Qingfang Deng @ 2026-09-07  6:16 UTC (permalink / raw)
  To: Vladislav Karmanov, netdev
  Cc: andrew, daniel, dqfext, SkyLake.Huang, hkallweit1, linux, davem,
	edumazet, kuba, pabeni, matthias.bgg, angelogioacchino.delregno,
	chester.a.unal, sean.wang, olteanv, yangshiji66, linux-kernel,
	linux-arm-kernel, linux-mediatek

On 2026/9/5 4:28, Vladislav Karmanov wrote:
> The MT7530 internal GE PHY advertises EEE by hardware default, but its
> EEE support is defective: with EEE advertised, some link partners fail
> to establish a stable link. On a 2-pair (4-wire) cable where both ends
> advertise gigabit, 1000BASE-T training cannot succeed, and instead of
> falling back to 100 Mbps the port loops, so no link or DHCP lease is
> ever obtained. MediaTek confirms the hardware is the root cause (Landen
> Chao, 2021): "EEE of the 10-year-old MT7530 internal gephy has many IOT
> problems, so it is recommended to disable its EEE."
>
> mtk_gephy_config_init() used to clear the EEE advertisement early, but
> commit af3b4b0e59de ("net: phy: mediatek-ge: do not disable EEE
> advertisement") removed that on the rationale that the DSA subdriver
> already performs an early disable. That holds for MT7531, whose
> mt7531_setup() clears MDIO_AN_EEE_ADV on each switch PHY, but not for
> the MT7530 PHY: neither the MT7621 integrated switch nor the dedicated
> MT7530 IC ever had such a loop, so removing it left those boards
> without any working early EEE disable and the link flapping came back.
>
> Since the broken hardware is the PHY, fix it in the PHY driver so it
> covers all users of this PHY, integrated in a switch or standalone:
>
>    - clear MDIO_AN_EEE_ADV in probe(), before anything can negotiate
>      EEE with the link partner;
>    - call phy_disable_eee() so phylib does not write the advertisement
>      back on later renegotiations and userspace cannot re-enable EEE.
>
> Auto-negotiation then falls back to a stable 100 Mbps link instead of
> looping at gigabit. Tested on ASUS RT-AX53U (MT7621): with a 2-pair
> cable on the WAN port, a single clean 100 Mbps link comes up and a
> DHCP lease is obtained, where the unpatched driver loops.
>
> Fixes: af3b4b0e59de ("net: phy: mediatek-ge: do not disable EEE advertisement")
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Vladislav Karmanov <vladislav.karmanov.dev@gmail.com>

Acked-by: Qingfang Deng <qingfang.deng@linux.dev>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v3] net: phy: mediatek-ge: disable EEE on the MT7530 PHY
  2026-09-04 20:28 [PATCH net v3] net: phy: mediatek-ge: disable EEE on the MT7530 PHY Vladislav Karmanov
  2026-09-04 20:34 ` Andrew Lunn
  2026-09-07  6:16 ` Qingfang Deng
@ 2026-09-08  8:30 ` netdev-bot+sashiko
  2026-09-08 15:14   ` Vladislav Karmanov
  2026-10-12 16:35 ` Bjørn Mork
  3 siblings, 1 reply; 6+ messages in thread
From: netdev-bot+sashiko @ 2026-09-08  8:30 UTC (permalink / raw)
  To: vladislav.karmanov.dev
  Cc: netdev, andrew, daniel, dqfext, SkyLake.Huang, hkallweit1, linux,
	davem, edumazet, kuba, pabeni, matthias.bgg,
	angelogioacchino.delregno, chester.a.unal, sean.wang, olteanv,
	yangshiji66, linux-kernel, linux-arm-kernel, linux-mediatek

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.

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v3] net: phy: mediatek-ge: disable EEE on the MT7530 PHY
  2026-09-08  8:30 ` netdev-bot+sashiko
@ 2026-09-08 15:14   ` Vladislav Karmanov
  0 siblings, 0 replies; 6+ messages in thread
From: Vladislav Karmanov @ 2026-09-08 15:14 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: netdev, andrew, daniel, dqfext, SkyLake.Huang, hkallweit1, linux,
	davem, edumazet, kuba, pabeni, matthias.bgg,
	angelogioacchino.delregno, chester.a.unal, sean.wang, olteanv,
	yangshiji66, linux-kernel, linux-arm-kernel, linux-mediatek

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/

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, <netdev-bot+sashiko@kernel.org>:
>
> 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.
>
> > 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().
>
> > +
> > +     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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v3] net: phy: mediatek-ge: disable EEE on the MT7530 PHY
  2026-09-04 20:28 [PATCH net v3] net: phy: mediatek-ge: disable EEE on the MT7530 PHY Vladislav Karmanov
                   ` (2 preceding siblings ...)
  2026-09-08  8:30 ` netdev-bot+sashiko
@ 2026-10-12 16:35 ` Bjørn Mork
  3 siblings, 0 replies; 6+ messages in thread
From: Bjørn Mork @ 2026-10-12 16:35 UTC (permalink / raw)
  To: Vladislav Karmanov
  Cc: netdev, andrew, daniel, dqfext, SkyLake.Huang, hkallweit1, linux,
	davem, edumazet, kuba, pabeni, matthias.bgg,
	angelogioacchino.delregno, chester.a.unal, sean.wang, olteanv,
	yangshiji66, linux-kernel, linux-arm-kernel, linux-mediatek

Vladislav Karmanov <vladislav.karmanov.dev@gmail.com> writes:

> The MT7530 internal GE PHY advertises EEE by hardware default, but its
> EEE support is defective: with EEE advertised, some link partners fail
> to establish a stable link. On a 2-pair (4-wire) cable where both ends
> advertise gigabit, 1000BASE-T training cannot succeed, and instead of
> falling back to 100 Mbps the port loops, so no link or DHCP lease is
> ever obtained.

I realize that I'm too late at the party here, but I still want to state
this for the record:

I just tried to reproduce this bug on a Zyxel NR7101, using a cable with
pair 4 cut, connecting to the I219-LM controller in my laptop.  Without
success.

EEE enabled or not does not make any difference.  The link is negotiated
as 100Mb/s as expected, and it is completely stable.  Absolutely no
issues observed.  I even added a .link_change_notify callback with a
counter and a printk to the phy driver to make sure I didn't miss any
flapping.

MT7621 side:

root@OpenWrt:~# ethtool --show-eee lan
EEE settings for lan:
        EEE status: enabled - active
        Tx LPI: 30 (us)
        Supported EEE link modes:  100baseT/Full
                                   1000baseT/Full
        Advertised EEE link modes:  100baseT/Full
                                    1000baseT/Full
        Link partner advertised EEE link modes:  100baseT/Full
                                                 1000baseT/Full
root@OpenWrt:~# ethtool lan
Settings for lan:
        Supported ports: [ TP    MII ]
        Supported link modes:   10baseT/Half 10baseT/Full
                                100baseT/Half 100baseT/Full
                                1000baseT/Full
        Supported pause frame use: Symmetric Receive-only
        Supports auto-negotiation: Yes
        Supported FEC modes: Not reported
        Advertised link modes:  10baseT/Half 10baseT/Full
                                100baseT/Half 100baseT/Full
                                1000baseT/Full
        Advertised pause frame use: Symmetric Receive-only
        Advertised auto-negotiation: Yes
        Advertised FEC modes: Not reported
        Link partner advertised link modes:  10baseT/Half 10baseT/Full
                                             100baseT/Half 100baseT/Full
        Link partner advertised pause frame use: No
        Link partner advertised auto-negotiation: Yes
        Link partner advertised FEC modes: Not reported
        Speed: 100Mb/s
        Duplex: Full
        Auto-negotiation: on
        master-slave cfg: preferred master
        master-slave status: master
        Port: Twisted Pair
        PHYAD: 2
        Transceiver: external
        MDI-X: Unknown
        Supports Wake-on: d
        Wake-on: d
        Link detected: yes

I219-LM side:

root@miraculix:/tmp# ethtool --show-eee eth0
EEE settings for eth0:
        EEE status: enabled - active
        Tx LPI: 17 (us)
        Supported EEE link modes:  100baseT/Full
                                   1000baseT/Full
        Advertised EEE link modes:  100baseT/Full
                                    1000baseT/Full
        Link partner advertised EEE link modes:  100baseT/Full
                                                 1000baseT/Full
root@miraculix:/tmp# ethtool eth0
Settings for eth0:
        Supported ports: [ TP ]
        Supported link modes:   10baseT/Half 10baseT/Full
                                100baseT/Half 100baseT/Full
                                1000baseT/Full
        Supported pause frame use: Symmetric Receive-only
        Supports auto-negotiation: Yes
        Supported FEC modes: Not reported
        Advertised link modes:  10baseT/Half 10baseT/Full
                                100baseT/Half 100baseT/Full
                                1000baseT/Full
        Advertised pause frame use: Symmetric Receive-only
        Advertised auto-negotiation: Yes
        Advertised FEC modes: Not reported
        Link partner advertised link modes:  10baseT/Half 10baseT/Full
                                             100baseT/Half 100baseT/Full
        Link partner advertised pause frame use: Symmetric Receive-only
        Link partner advertised auto-negotiation: Yes
        Link partner advertised FEC modes: Not reported
        Speed: 100Mb/s
        Duplex: Full
        Auto-negotiation: on
        Port: Twisted Pair
        PHYAD: 2
        Transceiver: internal
        MDI-X: on (auto)
        Supports Wake-on: pumbg
        Wake-on: d
        Current message level: 0x00000007 (7)
                               drv probe link
        Link detected: yes


So now I am curious to know:  What am I doing wrong here?

I primarily did this test because I wanted to implement a smarter
workaround, without forcibly disabling EEE for everyone based on one
pretty obscure configuration.  But that's hard without being able to
reproduce the problem.

So let me ask you, who seem to be able to reproduce it:  Why is it
necessary to block EEE for everyone? Why can't you disable EEE by
default, but let me enable it on my systems - where I know it works?

Or even better:  Why can't you detect the link flaps and only disable
EEE when required? (this was my plan)

FWIW, I have been using EEE on MT7621 devices for years without ever
noticing the claimed issue.  I have heard the rumours that the SoC is
supposed to have occasional link issues, and that they are related to
EEE.  But reproducing the issue has so far been impossible.

> MediaTek confirms the hardware is the root cause (Landen
> Chao, 2021): "EEE of the 10-year-old MT7530 internal gephy has many IOT
> problems, so it is recommended to disable its EEE."

This is BS. I don't doubt that there are bugs in the MT7530 gephy. But
the EEE feature of the MT7530 internal gephy works fine for me.  And
always has.

Vendors have no interest in making older hardware work. There is no
reason to ask them or trust them when it comes to hardware they don't
sell anymore.

We can do better. And we have, by for example bringing DSA to the
MT7621. Please, let's try to make even old hardware work as good as
possible.  Disabling features is not the way to go.

> Auto-negotiation then falls back to a stable 100 Mbps link instead of
> looping at gigabit. Tested on ASUS RT-AX53U (MT7621): with a 2-pair
> cable on the WAN port, a single clean 100 Mbps link comes up and a
> DHCP lease is obtained, where the unpatched driver loops.

And no I've tested the same configuration with EEE enabled, and it
doesn't loop either.

Where does that put us?


Bjørn

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-12 16:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-04 20:28 [PATCH net v3] net: phy: mediatek-ge: disable EEE on the MT7530 PHY Vladislav Karmanov
2026-09-04 20:34 ` Andrew Lunn
2026-09-07  6:16 ` Qingfang Deng
2026-09-08  8:30 ` netdev-bot+sashiko
2026-09-08 15:14   ` Vladislav Karmanov
2026-10-12 16:35 ` Bjørn Mork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®