Re: [PATCH net 1/2] net: dsa: mt7530: populate lpi_interfaces to fix EEE support

[Andrew Lunn]

On Sat, Aug 22, 2026 at 10:52:51PM +0300, Aleksei Sviridkin wrote:
> mt753x_phylink_get_caps() fills in config->lpi_capabilities and
> config->lpi_timer_default, but never populates config->lpi_interfaces.
> phylink only treats a MAC as supporting phylink managed EEE when the
> tx_lpi methods are implemented and both the LPI capabilities and the
> LPI interfaces are non-empty, so EEE is unavailable on every port:
> 
>   # ethtool --show-eee lan1
>   Cannot get EEE settings: Not supported
> 
> even though the driver implements mac_enable_tx_lpi() and
> mac_disable_tx_lpi() and reads the LPI threshold back out of PMEEECR.
> Since the tx_lpi methods are implemented, phylink takes the other
> branch and calls phy_disable_eee(), which fills eee_disabled_modes, so
> userspace cannot enable EEE either.
> 
> Only the first half of what commit 06dfcd4098cf ("net: dsa: mt7530: fix
> enabling EEE on MT7531 switch on all boards") arranged therefore
> survives. It left EEE off out of the box on purpose, by having
> mt7531_setup() clear the switch PHYs' EEE advertisement, and ended
> "With this change, EEE can now be enabled using ethtool". It cannot be,
> any more.
> 
> Copy the supported interfaces into lpi_interfaces. This requires moving
> the mac_port_get_caps() call ahead of the EEE block, since that is what
> populates supported_interfaces - copying it beforehand would copy an
> empty bitmap.
> 
> LPI stays off by default. The driver does not set eee_enabled_default,
> so phylink leaves tx_lpi_enabled false, and phy_check_link_status()
> computes enable_tx_lpi as tx_lpi_enabled && eee_active - nothing
> asserts LPI until userspace enables it with ethtool --set-eee. The EEE
> advertisement is the part that does change: phylink no longer takes the
> phy_disable_eee() branch, so a PHY that advertises EEE out of reset
> advertises it again and the link may negotiate EEE. MT7531's five
> internal PHYs are the exception, as mt7531_setup() zeroes
> MDIO_AN_EEE_ADV before the switch MDIO bus is registered, so
> phy_probe() reads an empty advertisement and records eee_cfg.eee_enabled
> as false. Nothing does that for an external PHY on port 5 or 6, or on
> the other mt753x variants, EN7528 aside - see below.
> 
> This also makes lpi_capabilities take effect for the first time, so
> correct its value in the same change. PMCR only has force bits for
> 100 Mbps (PMCR_FORCE_EEE100) and 1 Gbps (PMCR_FORCE_EEE1G), and PMSR
> only reports EEE state for those two speeds, so the MAC cannot signal
> LPI at 2.5 Gbps: drop MAC_2500FD. Absence from the header is weak
> evidence on its own, so for what it is worth, the Airoha AN8855 DSA
> driver - posted but not merged [1] - describes a PMCR of the same shape
> that does carry AN8855_PMCR_FORCE_EEE2P5G and AN8855_PMCR_FORCE_EEE5G
> next to the 1 Gbps and 100 Mbps bits. Correcting the value here rather
> than in a separate patch changes nothing observable: while
> lpi_interfaces was empty, lpi_capabilities never reached
> phy->advertising_eee, so no state ever claimed 2.5 Gbps LPI.
> 
> 2500BASE-X has to come out of lpi_interfaces as well, because
> lpi_capabilities cannot express it: it masks the PHY's EEE
> advertisement, a media side property, and never gates LPI activation on
> the MAC side speed. phylink raises the MAC speed to the interface
> maximum when the PHY rate matches (RATE_MATCH_PAUSE in
> phylink_link_up()), so a 1 Gbps media link behind a rate matching 2.5G
> PHY would otherwise arm LPI while the MAC runs at 2.5 Gbps. What that
> costs is limited to setups that keep the MAC on 2500BASE-X, where there
> are no LPI bits to use anyway; a PHY that switches the interface down
> to SGMII or 1000BASE-X keeps LPI, as those stay in the mask.
> 
> For the same reason, skip ports that support neither 100 Mbps nor
> 1 Gbps: on MT7988, EN7581 and AN7583, port 6 is 10 Gbps only, and it
> shares PHY_INTERFACE_MODE_INTERNAL with the 1 Gbps user ports, so the
> interface mask alone cannot tell them apart.
> 
> EEE remains unavailable on EN7528, whose GPHYs do not negotiate it
> reliably. Both LPI bitmaps stay empty there, so phylink keeps taking
> the phy_disable_eee() branch and its advertisement stays off.

This is very verbose. Is it AI generated? Please revise it and make it
suitable for a human. The important part of the commit message is
"Why?", since "What?" can be answered by reading the code.

> - mt753x_phylink_mac_enable_tx_lpi() sets the PMCR force-EEE bits
>   without checking the resolved speed or interface, relying entirely on
>   phylink never calling it above 1 Gbps. A check there would make the
>   driver robust independently of lpi_interfaces being right;
>   deliberately not bundled into a fix.

It is normal to assume phylink is correct. Now, if you do find it
doing the wrong thing, we want to know so we can fix it. So if you do
want to validate it here, make it very noisy if the validation fails.

	
    Andrew

---
pw-bot: cr