Subject: [PATCH RFC net-next] net: phy: marvell: add comment about m88e1111_config_init_1000basex()

From: "Russell King (Oracle)" <[email-masked]>
Date: Tue, 27 Feb 2024 15:59:50 +0000

The comment in m88e1111_config_init_1000basex() is wrong - it claims
that Autoneg will be enabled, but this doesn't actually happen.

Signed-off-by: Russell King (Oracle) <[email-masked]>
---
The 88E1111 datasheet says that when changing HWCFG to 1100 (1000base-X
no AN) then one also needs to clear bit 12 in the fibre BMCR.
Furthermore, it says that in the Mode Switching chapter that when
switching the mode between 1000 and 1100, bit 12 of the fibre BMCR needs
to be manually updated.

However, the 88E1111 code doesn't do this, it just changes HWCFG and then
seems to hope for the best. Is this something which should be fixed?

 drivers/net/phy/marvell.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 1faa22f58366..42ed013385bf 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -919,7 +919,10 @@ static int m88e1111_config_init_1000basex(struct phy_device *phydev)
 	if (extsr < 0)
 		return extsr;
 
-	/* If using copper mode, ensure 1000BaseX auto-negotiation is enabled */
+	/* If using copper mode, ensure 1000BaseX auto-negotiation is enabled.
+	 * FIXME: this does not actually enable 1000BaseX auto-negotiation if
+	 * it was previously disabled in the Fiber BMCR!
+	 */
 	mode = extsr & MII_M1111_HWCFG_MODE_MASK;
 	if (mode == MII_M1111_HWCFG_MODE_COPPER_1000X_NOAN) {
 		err = phy_modify(phydev, MII_M1111_PHY_EXT_SR,
-- 
2.30.2