net: marvell: mvpp2: only reprogram what is necessary on mac_config

mac_config() can be called at any point, and the expected behaviour
from MAC drivers is to only reprogram when necessary - and certainly
avoid taking the link down on every call.

Unfortunately, mvpp2 does exactly that - it takes the link down, and
reprograms everything, and then releases the forced-link down.

This is bad, it can cause the link to bounce:

- SFP detects signal, disables LOS indication.
- SFP code calls into phylink, calling phylink_sfp_link_up() which
  triggers a resolve.
- phylink_resolve() calls phylink_get_mac_state() and finds the MAC
  reporting link up.
- phylink wants to configure the pause mode on the MAC, so calls
  phylink_mac_config()
- mvpp2 takes the link down temporarily, generating a MAC link down
  event followed by another MAC link event.
- phylink calls mac_link_up() and then processes the MAC link down
  event.
- phylink_resolve() gets called again, registers the link down, and
  calls mach_link_down() before re-running itself.
- phylink_resolve() starts again at step 3 above.  This sequence
  repeats.

GMAC versions prior to mvpp2 do not require the link to be taken down
except when certain link properties (eg, switching between SGMII and
1000base-X mode, or enabling/disabling in-band negotiation) are
changed.  Implement this for mvpp2.

Tested-by: Sven Auhagen <sven.auhagen@voleatech.de>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 7a60456d91beb6330d3e4fc8169374f701c5871a..133a6d4f94a13a256f3d0d2b9e4b5e2a9787c092 100644 (file)
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4534,29 +4534,21 @@ static void mvpp2_xlg_config(struct mvpp2_port *port, unsigned int mode,
 static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
                              const struct phylink_link_state *state)
 {
-       u32 an, ctrl0, ctrl2, ctrl4;
-       u32 old_ctrl2;
+       u32 old_an, an;
+       u32 old_ctrl0, ctrl0;
+       u32 old_ctrl2, ctrl2;
+       u32 old_ctrl4, ctrl4;
 
-       an = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
-       ctrl0 = readl(port->base + MVPP2_GMAC_CTRL_0_REG);
-       ctrl2 = readl(port->base + MVPP2_GMAC_CTRL_2_REG);
-       ctrl4 = readl(port->base + MVPP22_GMAC_CTRL_4_REG);
-
-       old_ctrl2 = ctrl2;
-
-       /* Force link down */
-       an &= ~MVPP2_GMAC_FORCE_LINK_PASS;
-       an |= MVPP2_GMAC_FORCE_LINK_DOWN;
-       writel(an, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
-
-       /* Set the GMAC in a reset state */
-       ctrl2 |= MVPP2_GMAC_PORT_RESET_MASK;
-       writel(ctrl2, port->base + MVPP2_GMAC_CTRL_2_REG);
+       old_an = an = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+       old_ctrl0 = ctrl0 = readl(port->base + MVPP2_GMAC_CTRL_0_REG);
+       old_ctrl2 = ctrl2 = readl(port->base + MVPP2_GMAC_CTRL_2_REG);
+       old_ctrl4 = ctrl4 = readl(port->base + MVPP22_GMAC_CTRL_4_REG);
 
        an &= ~(MVPP2_GMAC_CONFIG_MII_SPEED | MVPP2_GMAC_CONFIG_GMII_SPEED |
                MVPP2_GMAC_AN_SPEED_EN | MVPP2_GMAC_FC_ADV_EN |
                MVPP2_GMAC_FC_ADV_ASM_EN | MVPP2_GMAC_FLOW_CTRL_AUTONEG |
-               MVPP2_GMAC_CONFIG_FULL_DUPLEX | MVPP2_GMAC_AN_DUPLEX_EN);
+               MVPP2_GMAC_CONFIG_FULL_DUPLEX | MVPP2_GMAC_AN_DUPLEX_EN |
+               MVPP2_GMAC_IN_BAND_AUTONEG | MVPP2_GMAC_IN_BAND_AUTONEG_BYPASS);
        ctrl0 &= ~MVPP2_GMAC_PORT_TYPE_MASK;
        ctrl2 &= ~(MVPP2_GMAC_INBAND_AN_MASK | MVPP2_GMAC_PORT_RESET_MASK |
                   MVPP2_GMAC_PCS_ENABLE_MASK);
@@ -4623,7 +4615,8 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
                 */
                ctrl0 |= MVPP2_GMAC_PORT_TYPE_MASK;
                an &= ~(MVPP2_GMAC_FORCE_LINK_DOWN | MVPP2_GMAC_FORCE_LINK_PASS);
-               an |= MVPP2_GMAC_CONFIG_GMII_SPEED |
+               an |= MVPP2_GMAC_IN_BAND_AUTONEG |
+                     MVPP2_GMAC_CONFIG_GMII_SPEED |
                      MVPP2_GMAC_CONFIG_FULL_DUPLEX;
 
                if (state->pause & MLO_PAUSE_AN && state->an_enabled) {
@@ -4636,10 +4629,29 @@ static void mvpp2_gmac_config(struct mvpp2_port *port, unsigned int mode,
                }
        }
 
-       writel(ctrl0, port->base + MVPP2_GMAC_CTRL_0_REG);
-       writel(ctrl2, port->base + MVPP2_GMAC_CTRL_2_REG);
-       writel(ctrl4, port->base + MVPP22_GMAC_CTRL_4_REG);
-       writel(an, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+       if ((old_ctrl0 ^ ctrl0) & MVPP2_GMAC_PORT_TYPE_MASK ||
+           (old_ctrl2 ^ ctrl2) & MVPP2_GMAC_INBAND_AN_MASK ||
+           (old_an ^ an) & MVPP2_GMAC_IN_BAND_AUTONEG) {
+               /* Force link down */
+               old_an &= ~MVPP2_GMAC_FORCE_LINK_PASS;
+               old_an |= MVPP2_GMAC_FORCE_LINK_DOWN;
+               writel(old_an, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
+
+               /* Set the GMAC in a reset state - do this in a way that
+                * ensures we clear it below.
+                */
+               old_ctrl2 |= MVPP2_GMAC_PORT_RESET_MASK;
+               writel(old_ctrl2, port->base + MVPP2_GMAC_CTRL_2_REG);
+       }
+
+       if (old_ctrl0 != ctrl0)
+               writel(ctrl0, port->base + MVPP2_GMAC_CTRL_0_REG);
+       if (old_ctrl2 != ctrl2)
+               writel(ctrl2, port->base + MVPP2_GMAC_CTRL_2_REG);
+       if (old_ctrl4 != ctrl4)
+               writel(ctrl4, port->base + MVPP22_GMAC_CTRL_4_REG);
+       if (old_an != an)
+               writel(an, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
 
        if (old_ctrl2 & MVPP2_GMAC_PORT_RESET_MASK) {
                while (readl(port->base + MVPP2_GMAC_CTRL_2_REG) &