diff mbox series

[net-next] net: phy: marvell: Ensure SGMII auto-negotiation is enabled for 88E1111

Message ID 20210213002629.2557315-1-robert.hancock@calian.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: phy: marvell: Ensure SGMII auto-negotiation is enabled for 88E1111 | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: line length of 87 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Robert Hancock Feb. 13, 2021, 12:26 a.m. UTC
When 88E111 is operating in SGMII mode, auto-negotiation should be enabled
on the SGMII side so that the link will come up properly with PCSes which
normally have auto-negotiation enabled. This is normally the case when the
PHY defaults to SGMII mode at power-up, however if we switched it from some
other mode like 1000BaseX, as may happen in some SFP module situations,
it may not be.

Call genphy_check_and_restart_aneg on the fiber page to ensure that auto-
negotiation is properly enabled on the SGMII interface.

Signed-off-by: Robert Hancock <robert.hancock@calian.com>
---
 drivers/net/phy/marvell.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Russell King (Oracle) Feb. 13, 2021, 1:09 a.m. UTC | #1
On Fri, Feb 12, 2021 at 06:26:29PM -0600, Robert Hancock wrote:
> When 88E111 is operating in SGMII mode, auto-negotiation should be enabled

88E1111.

> on the SGMII side so that the link will come up properly with PCSes which
> normally have auto-negotiation enabled. This is normally the case when the
> PHY defaults to SGMII mode at power-up, however if we switched it from some
> other mode like 1000BaseX, as may happen in some SFP module situations,
> it may not be.

Do you actually have a module where this applies?

I have modules that do come up in 1000base-X mode, but do switch to
SGMII mode with AN just fine. So I'm wondering what the difference is.
Robert Hancock Feb. 13, 2021, 1:28 a.m. UTC | #2
On Sat, 2021-02-13 at 01:09 +0000, Russell King - ARM Linux admin wrote:
> On Fri, Feb 12, 2021 at 06:26:29PM -0600, Robert Hancock wrote:
> > When 88E111 is operating in SGMII mode, auto-negotiation should be enabled
> 
> 88E1111.

yup..

> 
> > on the SGMII side so that the link will come up properly with PCSes which
> > normally have auto-negotiation enabled. This is normally the case when the
> > PHY defaults to SGMII mode at power-up, however if we switched it from some
> > other mode like 1000BaseX, as may happen in some SFP module situations,
> > it may not be.
> 
> Do you actually have a module where this applies?
> 
> I have modules that do come up in 1000base-X mode, but do switch to
> SGMII mode with AN just fine. So I'm wondering what the difference is.

I saw this with a Finisar FCLF8520P2BTL, which defaults to 1000Base-X with
host-side auto-negotiation disabled. So presumably the auto-negotiation
disabled state carries over when it is switched to SGMII mode. I previously
wrote a patch ("net: phy: marvell: add special handling of Finisar modules with
88E1111") which enabled auto-negotiation for 1000Base-X mode, but we are trying
to switch over to using SGMII with these now.

>
diff mbox series

Patch

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 3238d0fbf437..e26a5d663f8a 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -684,16 +684,19 @@  static int m88e1111_config_aneg(struct phy_device *phydev)
 	if (err < 0)
 		goto error;
 
-	/* Do not touch the fiber page if we're in copper->sgmii mode */
-	if (phydev->interface == PHY_INTERFACE_MODE_SGMII)
-		return 0;
-
 	/* Then the fiber link */
 	err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE);
 	if (err < 0)
 		goto error;
 
-	err = marvell_config_aneg_fiber(phydev);
+	if (phydev->interface == PHY_INTERFACE_MODE_SGMII)
+		/* Do not touch the fiber advertisement if we're in copper->sgmii mode.
+		 * Just ensure that SGMII-side autonegotiation is enabled.
+		 * If we switched from some other mode to SGMII it may not be.
+		 */
+		err = genphy_check_and_restart_aneg(phydev, false);
+	else
+		err = marvell_config_aneg_fiber(phydev);
 	if (err < 0)
 		goto error;