net: phylink: re-implement interface configuration with PCS

With PCS support, how we implement interface reconfiguration (or other
major reconfiguration) is not up to the job; we end up reconfiguring
the PCS for an interface change while the link could potentially be up.
In order to solve this, add two additional MAC methods for major
configuration, one to prepare for the change, and one to finish the
change.

This allows mvneta and mvpp2 to shutdown what they require prior to the
MAC and PCS configuration calls, and then restart as appropriate.

This impacts ksettings_set(), which now needs to identify whether the
change is a minor tweak to the advertisement masks or whether the
interface mode has changed, and call the appropriate function for that
update.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 84a4264011028348e2abd4f61ec673dd22b51979..d554a0fbb4f38112563aeea91384195513a7a04f 100644 (file)
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -433,23 +433,47 @@ static void phylink_mac_pcs_an_restart(struct phylink *pl)
        }
 }
 
-static void phylink_pcs_config(struct phylink *pl, bool force_restart,
-                              const struct phylink_link_state *state)
+static void phylink_major_config(struct phylink *pl, bool restart,
+                                 const struct phylink_link_state *state)
 {
-       bool restart = force_restart;
+       int err;
+
+       phylink_dbg(pl, "major config %s\n", phy_modes(state->interface));
 
-       if (pl->pcs_ops && pl->pcs_ops->pcs_config(pl->config,
-                                                  pl->cur_link_an_mode,
-                                                  state->interface,
-                                                  state->advertising,
-                                                  !!(pl->link_config.pause &
-                                                     MLO_PAUSE_AN)))
-               restart = true;
+       if (pl->mac_ops->mac_prepare) {
+               err = pl->mac_ops->mac_prepare(pl->config, pl->cur_link_an_mode,
+                                              state->interface);
+               if (err < 0) {
+                       phylink_err(pl, "mac_prepare failed: %pe\n",
+                                   ERR_PTR(err));
+                       return;
+               }
+       }
 
        phylink_mac_config(pl, state);
 
+       if (pl->pcs_ops) {
+               err = pl->pcs_ops->pcs_config(pl->config, pl->cur_link_an_mode,
+                                             state->interface,
+                                             state->advertising,
+                                             !!(pl->link_config.pause &
+                                                MLO_PAUSE_AN));
+               if (err < 0)
+                       phylink_err(pl, "pcs_config failed: %pe\n",
+                                   ERR_PTR(err));
+               if (err > 0)
+                       restart = true;
+       }
        if (restart)
                phylink_mac_pcs_an_restart(pl);
+
+       if (pl->mac_ops->mac_finish) {
+               err = pl->mac_ops->mac_finish(pl->config, pl->cur_link_an_mode,
+                                             state->interface);
+               if (err < 0)
+                       phylink_err(pl, "mac_prepare failed: %pe\n",
+                                   ERR_PTR(err));
+       }
 }
 
 /*
@@ -555,7 +579,7 @@ static void phylink_mac_initial_config(struct phylink *pl, bool force_restart)
        link_state.link = false;
 
        phylink_apply_manual_flow(pl, &link_state);
-       phylink_pcs_config(pl, force_restart, &link_state);
+       phylink_major_config(pl, force_restart, &link_state);
 }
 
 static const char *phylink_pause_to_str(int pause)
@@ -674,7 +698,7 @@ static void phylink_resolve(struct work_struct *w)
                                phylink_link_down(pl);
                                cur_link_state = false;
                        }
-                       phylink_pcs_config(pl, false, &link_state);
+                       phylink_major_config(pl, false, &link_state);
                        pl->link_config.interface = link_state.interface;
                } else if (!pl->pcs_ops) {
                        /* The interface remains unchanged, only the speed,
@@ -1450,21 +1474,26 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
                return -EINVAL;
 
        mutex_lock(&pl->state_mutex);
-       linkmode_copy(pl->link_config.advertising, config.advertising);
-       pl->link_config.interface = config.interface;
        pl->link_config.speed = config.speed;
        pl->link_config.duplex = config.duplex;
        pl->link_config.an_enabled = config.an_enabled;
 
-       if (pl->cur_link_an_mode == MLO_AN_INBAND &&
-           !test_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state)) {
-               /* If in 802.3z mode, this updates the advertisement.
-                *
-                * If we are in SGMII mode without a PHY, there is no
-                * advertisement; the only thing we have is the pause
-                * modes which can only come from a PHY.
-                */
-               phylink_pcs_config(pl, true, &pl->link_config);
+       if (pl->link_config.interface != config.interface) {
+               /* The interface changed, e.g. 1000base-X <-> 2500base-X */
+               /* We need to force the link down, then change the interface */
+               if (pl->old_link_state) {
+                       phylink_link_down(pl);
+                       pl->old_link_state = false;
+               }
+               if (!test_bit(PHYLINK_DISABLE_STOPPED,
+                             &pl->phylink_disable_state))
+                       phylink_major_config(pl, false, &config);
+               pl->link_config.interface = config.interface;
+               linkmode_copy(pl->link_config.advertising, config.advertising);
+       } else if (!linkmode_equal(pl->link_config.advertising,
+                                  config.advertising)) {
+               linkmode_copy(pl->link_config.advertising, config.advertising);
+               phylink_change_inband_advert(pl);
        }
        mutex_unlock(&pl->state_mutex);
 

diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index d9913d8e6b9144a7d7528cdd1608342edb65a4be..2f1315f32113778476029467b0cae98991bea4c5 100644 (file)
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -76,7 +76,9 @@ struct phylink_config {
  * struct phylink_mac_ops - MAC operations structure.
  * @validate: Validate and update the link configuration.
  * @mac_pcs_get_state: Read the current link state from the hardware.
+ * @mac_prepare: prepare for a major reconfiguration of the interface.
  * @mac_config: configure the MAC for the selected mode and state.
+ * @mac_finish: finish a major reconfiguration of the interface.
  * @mac_an_restart: restart 802.3z BaseX autonegotiation.
  * @mac_link_down: take the link down.
  * @mac_link_up: allow the link to come up.
@@ -89,8 +91,12 @@ struct phylink_mac_ops {
                         struct phylink_link_state *state);
        void (*mac_pcs_get_state)(struct phylink_config *config,
                                  struct phylink_link_state *state);
+       int (*mac_prepare)(struct phylink_config *config, unsigned int mode,
+                          phy_interface_t iface);
        void (*mac_config)(struct phylink_config *config, unsigned int mode,
                           const struct phylink_link_state *state);
+       int (*mac_finish)(struct phylink_config *config, unsigned int mode,
+                         phy_interface_t iface);
        void (*mac_an_restart)(struct phylink_config *config);
        void (*mac_link_down)(struct phylink_config *config, unsigned int mode,
                              phy_interface_t interface);
@@ -145,6 +151,31 @@ void validate(struct phylink_config *config, unsigned long *supported,
 void mac_pcs_get_state(struct phylink_config *config,
                       struct phylink_link_state *state);
 
+/**
+ * mac_prepare() - prepare to change the PHY interface mode
+ * @config: a pointer to a &struct phylink_config.
+ * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
+ * @iface: interface mode to switch to
+ *
+ * phylink will call this method at the beginning of a full initialisation
+ * of the link, which includes changing the interface mode or at initial
+ * startup time. It may be called for the current mode. The MAC driver
+ * should perform whatever actions are required, e.g. disabling the
+ * Serdes PHY.
+ *
+ * This will be the first call in the sequence:
+ * - mac_prepare()
+ * - mac_config()
+ * - pcs_config()
+ * - possible pcs_an_restart()
+ * - mac_finish()
+ *
+ * Returns zero on success, or negative errno on failure which will be
+ * reported to the kernel log.
+ */
+int mac_prepare(struct phylink_config *config, unsigned int mode,
+               phy_interface_t iface);
+
 /**
  * mac_config() - configure the MAC for the selected mode and state
  * @config: a pointer to a &struct phylink_config.
@@ -220,6 +251,23 @@ void mac_pcs_get_state(struct phylink_config *config,
 void mac_config(struct phylink_config *config, unsigned int mode,
                const struct phylink_link_state *state);
 
+/**
+ * mac_finish() - finish a to change the PHY interface mode
+ * @config: a pointer to a &struct phylink_config.
+ * @mode: one of %MLO_AN_FIXED, %MLO_AN_PHY, %MLO_AN_INBAND.
+ * @iface: interface mode to switch to
+ *
+ * phylink will call this if it called mac_prepare() to allow the MAC to
+ * complete any necessary steps after the MAC and PCS have been configured
+ * for the @mode and @iface. E.g. a MAC driver may wish to re-enable the
+ * Serdes PHY here if it was previously disabled by mac_prepare().
+ *
+ * Returns zero on success, or negative errno on failure which will be
+ * reported to the kernel log.
+ */
+int mac_finish(struct phylink_config *config, unsigned int mode,
+               phy_interface_t iface);
+
 /**
  * mac_an_restart() - restart 802.3z BaseX autonegotiation
  * @config: a pointer to a &struct phylink_config.