All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King <rmk+kernel@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Ioana Ciornei <ioana.ciornei@nxp.com>
Cc: Vladimir Oltean <vladimir.oltean@nxp.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Alexandru Marginean <alexandru.marginean@nxp.com>,
	"michael@walle.cc" <michael@walle.cc>,
	"olteanv@gmail.com" <olteanv@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org
Subject: [PATCH RFC net-next 11/13] net: phylink: re-implement interface configuration with PCS
Date: Tue, 30 Jun 2020 15:29:27 +0100	[thread overview]
Message-ID: <E1jqHGJ-0006QA-E2@rmk-PC.armlinux.org.uk> (raw)
In-Reply-To: <20200630142754.GC1551@shell.armlinux.org.uk>

With PCS support, how we implement interface 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 interface 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>
---
 drivers/net/phy/phylink.c | 80 ++++++++++++++++++++++++++-------------
 include/linux/phylink.h   | 48 +++++++++++++++++++++++
 2 files changed, 102 insertions(+), 26 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 09f4aeef15c7..a31a00fb4974 100644
--- 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_change_interface(struct phylink *pl, bool restart,
+				     const struct phylink_link_state *state)
 {
-	bool restart = force_restart;
+	int err;
+
+	phylink_dbg(pl, "change interface %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_change_interface(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_change_interface(pl, false, &link_state);
 			pl->link_config.interface = link_state.interface;
 		} else if (!pl->pcs_ops) {
 			/* The interface remains unchanged, only the speed,
@@ -1450,22 +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 = kset->base.autoneg !=
-				     AUTONEG_DISABLE;
-
-	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);
+	pl->link_config.an_enabled = kset->base.autoneg != AUTONEG_DISABLE;
+
+	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_change_interface(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 d9913d8e6b91..2f1315f32113 100644
--- 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.
-- 
2.20.1


  parent reply	other threads:[~2020-06-30 14:29 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-30 14:27 [PATCH RFC net-next 00/13] Phylink PCS updates Russell King - ARM Linux admin
2020-06-30 14:28 ` [PATCH RFC net-next 01/13] net: phylink: update ethtool reporting for fixed-link modes Russell King
2020-06-30 18:15   ` Florian Fainelli
2020-06-30 14:28 ` [PATCH RFC net-next 02/13] net: phylink: rejig link state tracking Russell King
2020-06-30 18:15   ` Florian Fainelli
2020-06-30 14:28 ` [PATCH RFC net-next 03/13] net: phylink: rearrange resolve mac_config() call Russell King
2020-06-30 18:32   ` Florian Fainelli
2020-06-30 14:28 ` [PATCH RFC net-next 04/13] net: phylink: ensure link is down when changing interface Russell King
2020-06-30 18:32   ` Florian Fainelli
2020-06-30 14:28 ` [PATCH RFC net-next 05/13] net: phylink: update PCS when changing interface during resolution Russell King
2020-06-30 18:33   ` Florian Fainelli
2020-06-30 14:29 ` [PATCH RFC net-next 06/13] net: phylink: avoid mac_config calls Russell King
2020-06-30 19:04   ` Florian Fainelli
2020-06-30 14:29 ` [PATCH RFC net-next 07/13] net: phylink: simplify ksettings_set() implementation Russell King
2020-07-20 10:24   ` Ioana Ciornei
2020-06-30 14:29 ` [PATCH RFC net-next 08/13] net: phylink: simplify phy case for ksettings_set method Russell King
2020-07-20 10:44   ` Ioana Ciornei
2020-07-20 12:45     ` Russell King - ARM Linux admin
2020-06-30 14:29 ` [PATCH RFC net-next 09/13] net: phylink: simplify fixed-link " Russell King
2020-07-20 10:52   ` Ioana Ciornei
2020-06-30 14:29 ` [PATCH RFC net-next 10/13] net: phylink: in-band pause mode advertisement update for PCS Russell King
2020-06-30 16:19   ` Jakub Kicinski
2020-06-30 14:29 ` Russell King [this message]
2020-07-20 11:40   ` [PATCH RFC net-next 11/13] net: phylink: re-implement interface configuration with PCS Ioana Ciornei
2020-07-20 12:44     ` Russell King - ARM Linux admin
2020-07-20 13:02       ` Ioana Ciornei
2020-07-20 14:19         ` Russell King - ARM Linux admin
2020-06-30 14:29 ` [PATCH RFC net-next 12/13] net: phylink: add struct phylink_pcs Russell King
2020-07-01 10:47   ` Vladimir Oltean
2020-07-01 11:16     ` Russell King - ARM Linux admin
2020-07-01 11:24       ` Vladimir Oltean
2020-07-20 15:50   ` Ioana Ciornei
2020-07-20 16:26     ` Russell King - ARM Linux admin
2020-07-20 16:59       ` Ioana Ciornei
2020-07-20 18:16         ` Russell King - ARM Linux admin
2020-06-30 14:29 ` [PATCH RFC net-next 13/13] net: phylink: add interface to configure clause 22 PCS PHY Russell King
2020-07-01 10:52   ` Ioana Ciornei
2020-07-14  8:49 ` [PATCH RFC net-next 00/13] Phylink PCS updates Vladimir Oltean
2020-07-14 13:18   ` Russell King - ARM Linux admin
2020-07-14 21:22     ` Florian Fainelli
2020-07-15  9:53       ` Russell King - ARM Linux admin
2020-07-14 23:46     ` Vladimir Oltean
2020-07-15 11:21       ` Russell King - ARM Linux admin
2020-07-15 11:34         ` Russell King - ARM Linux admin
2020-07-15 12:31           ` Vladimir Oltean
2020-07-15 14:20             ` Andrew Lunn
2020-07-15 17:02             ` Russell King - ARM Linux admin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E1jqHGJ-0006QA-E2@rmk-PC.armlinux.org.uk \
    --to=rmk+kernel@armlinux.org.uk \
    --cc=alexandru.marginean@nxp.com \
    --cc=andrew@lunn.ch \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=ioana.ciornei@nxp.com \
    --cc=kuba@kernel.org \
    --cc=michael@walle.cc \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=vladimir.oltean@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.