On Mon, Oct 27, 2025 at 01:57:48PM +0100, Emanuele Ghidoli wrote:
On 27/10/2025 00:45, Andrew Lunn wrote:
Since the introduction of phylink-managed EEE support in the stmmac driver, EEE is now enabled by default, leading to issues on systems using the DP83867 PHY.
Did you do a bisect to prove this?
Yes, I have done a bisect and the commit that introduced the behavior on our board is 4218647d4556 ("net: stmmac: convert to phylink managed EEE support").
Fixes: 2a10154abcb7 ("net: phy: dp83867: Add TI dp83867 phy")
What has this Fixes: tag got to do with phylink?
I think that the phylink commit is just enabling by default the EEE support, and my commit is not really fixing that. It is why I didn't put a Fixes: tag pointing to that.
I’ve tried to trace the behavior, but it’s quite complex. From my testing, I can summarize the situation as follows:
- ethtool, after that patch, returns:
ethtool --show-eee end0 EEE settings for end0: EEE status: enabled - active Tx LPI: 1000000 (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
- before that patch returns, after boot:
EEE settings for end0: EEE status: disabled Tx LPI: disabled Supported EEE link modes: 100baseT/Full 1000baseT/Full Advertised EEE link modes: Not reported Link partner advertised EEE link modes: 100baseT/Full 1000baseT/Full
Oh damn. I see why now:
/* Some DT bindings do not set-up the PHY handle. Let's try to * manually parse it */ if (!phy_fwnode || IS_ERR(phy_fwnode)) { int addr = priv->plat->phy_addr; ... if (priv->dma_cap.eee) phy_support_eee(phydev);
ret = phylink_connect_phy(priv->phylink, phydev); } else { fwnode_handle_put(phy_fwnode); ret = phylink_fwnode_phy_connect(priv->phylink, fwnode, 0); }
The driver only considers calling phy_support_eee() when DT fails to describe the PHY (because in the other path, it doesn't have access to the struct phy_device to make this call.)
My commit makes it apply even to DT described PHYs, so now (as has been shown when you enable EEE manually) it's uncovering latent problems.
So now we understand why the change has occurred - this is important. Now the question becomes, what to do about it.
For your issue, given that we have statements from TI that indicate none of their gigabit PHYs support EEE, we really should not be reporting to userspace that there is any EEE support. Therefore, "Supported EEE link modes" should be completely empty.
I think I understand why we're getting EEE modes supported. In the DP83867 manual, it states for the DEVAD field of the C45 indirect access registers:
"Device Address: In general, these bits [4:0] are the device address DEVAD that directs any accesses of ADDAR register (0x000E) to the appropriate MMD. Specifically, the DP83867 uses the vendor specific DEVAD [4:0] = 11111 for accesses. All accesses through registers REGCR and ADDAR can use this DEVAD. Transactions with other DEVAD are ignored."
Specifically, that last sentence, and the use of "ignored". If this means the PHY does not drive the MDIO data line when registers are read, they will return 0xffff, which is actually against the IEEE requirements for C45 registers (unimplemented C45 registers are supposed to be zero.)
So, this needs to be tested - please modify phylib's genphy_c45_read_eee_cap1() to print the value read from the register.
If it is 0xffff, that confirms that theory.
The correct solution here, to stop other MAC drivers running into this is for TI PHY drivers to implement the .get_features() phylib method, call genphy_read_abilities() or genphy_c45_pma_read_abilities() as appropriate, and then clear phydev->supported_eee so the PHY driver reports (correctly according to TI's statements) that EEE modes are not supported.
So, while my commit does have an unintended change of behaviour (thanks for helping to locate that), it would appear that it has revealed a latent bug in likely all TI PHY gigabit drivers that needs fixing independently of what we do about this.