[PATCH net-next] net: dsa: felix: Don't restart PCS SGMII AN if not needed

[PATCH net-next] net: dsa: felix: Don't restart PCS SGMII AN if not needed

[Vladimir Oltean]

From: Alex Marginean <alexandru.marginean@nxp.com>

Some PHYs like VSC8234 don't like it when AN restarts on their system side
and they restart line side AN too, going into an endless link up/down loop.
Don't restart PCS AN if link is up already.

Although in theory this feedback loop should be possible with the other
in-band AN modes too, for some reason it was not seen with the VSC8514
QSGMII and AQR412 USXGMII PHYs. So keep this logic only for SGMII where
the problem was found.

Fixes: bdeced75b13f ("net: dsa: felix: Add PCS operations for PHYLINK")
Suggested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix_vsc9959.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
index 1e82b0d72058..2c812b481778 100644
--- a/drivers/net/dsa/ocelot/felix_vsc9959.c
+++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
@@ -645,6 +645,27 @@ static void vsc9959_pcs_init_sgmii(struct phy_device *pcs,
 				   const struct phylink_link_state *state)
 {
 	if (link_an_mode == MLO_AN_INBAND) {
+		int bmsr, bmcr;
+
+		/* Some PHYs like VSC8234 don't like it when AN restarts on
+		 * their system  side and they restart line side AN too, going
+		 * into an endless link up/down loop.  Don't restart PCS AN if
+		 * link is up already.
+		 * We do check that AN is enabled just in case this is the 1st
+		 * call, PCS detects a carrier but AN is disabled from power on
+		 * or by boot loader.
+		 */
+		bmcr = phy_read(pcs, MII_BMCR);
+		if (bmcr < 0)
+			return;
+
+		bmsr = phy_read(pcs, MII_BMSR);
+		if (bmsr < 0)
+			return;
+
+		if ((bmcr & BMCR_ANENABLE) && (bmsr & BMSR_LSTATUS))
+			return;
+
 		/* SGMII spec requires tx_config_Reg[15:0] to be exactly 0x4001
 		 * for the MAC PCS in order to acknowledge the AN.
 		 */
-- 
2.17.1

Re: [PATCH net-next] net: dsa: felix: Don't restart PCS SGMII AN if not needed

[David Miller]

From: Vladimir Oltean <olteanv@gmail.com>
Date: Thu, 16 Jan 2020 20:09:59 +0200

> From: Alex Marginean <alexandru.marginean@nxp.com>
> 
> Some PHYs like VSC8234 don't like it when AN restarts on their system side
> and they restart line side AN too, going into an endless link up/down loop.
> Don't restart PCS AN if link is up already.
> 
> Although in theory this feedback loop should be possible with the other
> in-band AN modes too, for some reason it was not seen with the VSC8514
> QSGMII and AQR412 USXGMII PHYs. So keep this logic only for SGMII where
> the problem was found.
> 
> Fixes: bdeced75b13f ("net: dsa: felix: Add PCS operations for PHYLINK")
> Suggested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Applied.