[RFC PATCH v2 net-next 0/5] Let phylink manage in-band AN for the PHY

[RFC PATCH v2 net-next 0/5] Let phylink manage in-band AN for the PHY

[Vladimir Oltean]

This small series creates a configuration knob for PHY drivers which use
serial MII-side interfaces and support clause 37 in-band auto-negotiation
there.

Changes in v2:
Incorporated feedback from Russell, which was to consider PHYs on SFP
modules too, and unify phylink's detection of PHYs with broken in-band
autoneg with the newly introduced PHY driver methods.
https://patchwork.kernel.org/project/netdevbpf/cover/20210212172341.3489046-1-olteanv@gmail.com/

This change set is only superficially tested, hence the RFC tag. It does
what I need on the NXP boards with on-board PHYs that I have, and also
seems to behave the same as before when I use a 1G SGMII SFP module with
the Marvell 88E1111 PHY (the only thing I have). I do not have the
ability to test the Methode DM7052 SFP module for the bcm84881.c driver
change, since I don't have that.

Posting the patch series mostly to figure out whether I understood the
change request correctly.

Vladimir Oltean (5):
  net: phylink: pass the phy argument to phylink_sfp_config
  net: phylink: introduce a generic method for querying PHY in-band
    autoneg capability
  net: phy: bcm84881: move the in-band capability check where it belongs
  net: phylink: explicitly configure in-band autoneg for PHYs that
    support it
  net: phy: mscc: configure in-band auto-negotiation for VSC8514

 drivers/net/phy/bcm84881.c       | 10 ++++
 drivers/net/phy/mscc/mscc.h      |  2 +
 drivers/net/phy/mscc/mscc_main.c | 20 +++++++
 drivers/net/phy/phy.c            | 25 +++++++++
 drivers/net/phy/phylink.c        | 93 +++++++++++++++++++++++++-------
 include/linux/phy.h              | 24 +++++++++
 6 files changed, 154 insertions(+), 20 deletions(-)

-- 
2.25.1

[RFC PATCH v2 net-next 1/5] net: phylink: pass the phy argument to phylink_sfp_config

[Vladimir Oltean]

Problem statement: I would like to move the phy_no_inband() check inside
phylink_sfp_config(), right _after_ the PHY mode was determined by
sfp_select_interface(). But phylink_sfp_config() does not take the "phy"
as argument, only one of its callers (phylink_sfp_connect_phy) does.

phylink_sfp_config is called from:

- phylink_sfp_module_insert, if we know that the SFP module may not have
  a PHY
- phylink_sfp_module_start, if the SFP module may have a PHY but it is
  not available here (otherwise the "if (pl->phydev)" check right above
  would have triggered)
- phylink_sfp_connect_phy, which by definition has a PHY

So of all 3 callers, 2 are certain there is no PHY at that particular
moment, and 1 is certain there is one.

After further analysis, the "mode" is assumed to be MLO_AN_INBAND unless
there is a PHY, and that PHY has broken inband capabilities. So if we
pass the PHY pointer (be it NULL), we can drop the "mode" argument and
deduce it locally.

To avoid a forward-declaration, this change also moves phylink_phy_no_inband
above phylink_sfp_config.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/phylink.c | 39 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 2cdf9f989dec..28edb3665ee9 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2035,6 +2035,15 @@ int phylink_speed_up(struct phylink *pl)
 }
 EXPORT_SYMBOL_GPL(phylink_speed_up);
 
+/* The Broadcom BCM84881 in the Methode DM7052 is unable to provide a SGMII
+ * or 802.3z control word, so inband will not work.
+ */
+static bool phylink_phy_no_inband(struct phy_device *phy)
+{
+	return phy->is_c45 &&
+		(phy->c45_ids.device_ids[1] & 0xfffffff0) == 0xae025150;
+}
+
 static void phylink_sfp_attach(void *upstream, struct sfp_bus *bus)
 {
 	struct phylink *pl = upstream;
@@ -2049,7 +2058,7 @@ static void phylink_sfp_detach(void *upstream, struct sfp_bus *bus)
 	pl->netdev->sfp_bus = NULL;
 }
 
-static int phylink_sfp_config(struct phylink *pl, u8 mode,
+static int phylink_sfp_config(struct phylink *pl, struct phy_device *phy,
 			      const unsigned long *supported,
 			      const unsigned long *advertising)
 {
@@ -2057,6 +2066,7 @@ static int phylink_sfp_config(struct phylink *pl, u8 mode,
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(support);
 	struct phylink_link_state config;
 	phy_interface_t iface;
+	unsigned int mode;
 	bool changed;
 	int ret;
 
@@ -2086,6 +2096,11 @@ static int phylink_sfp_config(struct phylink *pl, u8 mode,
 		return -EINVAL;
 	}
 
+	if (phy && phylink_phy_no_inband(phy))
+		mode = MLO_AN_PHY;
+	else
+		mode = MLO_AN_INBAND;
+
 	config.interface = iface;
 	linkmode_copy(support1, support);
 	ret = phylink_validate(pl, support1, &config);
@@ -2148,7 +2163,7 @@ static int phylink_sfp_module_insert(void *upstream,
 	if (pl->sfp_may_have_phy)
 		return 0;
 
-	return phylink_sfp_config(pl, MLO_AN_INBAND, support, support);
+	return phylink_sfp_config(pl, NULL, support, support);
 }
 
 static int phylink_sfp_module_start(void *upstream)
@@ -2167,8 +2182,7 @@ static int phylink_sfp_module_start(void *upstream)
 	if (!pl->sfp_may_have_phy)
 		return 0;
 
-	return phylink_sfp_config(pl, MLO_AN_INBAND,
-				  pl->sfp_support, pl->sfp_support);
+	return phylink_sfp_config(pl, NULL, pl->sfp_support, pl->sfp_support);
 }
 
 static void phylink_sfp_module_stop(void *upstream)
@@ -2199,20 +2213,10 @@ static void phylink_sfp_link_up(void *upstream)
 	phylink_run_resolve(pl);
 }
 
-/* The Broadcom BCM84881 in the Methode DM7052 is unable to provide a SGMII
- * or 802.3z control word, so inband will not work.
- */
-static bool phylink_phy_no_inband(struct phy_device *phy)
-{
-	return phy->is_c45 &&
-		(phy->c45_ids.device_ids[1] & 0xfffffff0) == 0xae025150;
-}
-
 static int phylink_sfp_connect_phy(void *upstream, struct phy_device *phy)
 {
 	struct phylink *pl = upstream;
 	phy_interface_t interface;
-	u8 mode;
 	int ret;
 
 	/*
@@ -2224,13 +2228,8 @@ static int phylink_sfp_connect_phy(void *upstream, struct phy_device *phy)
 	 */
 	phy_support_asym_pause(phy);
 
-	if (phylink_phy_no_inband(phy))
-		mode = MLO_AN_PHY;
-	else
-		mode = MLO_AN_INBAND;
-
 	/* Do the initial configuration */
-	ret = phylink_sfp_config(pl, mode, phy->supported, phy->advertising);
+	ret = phylink_sfp_config(pl, phy, phy->supported, phy->advertising);
 	if (ret < 0)
 		return ret;
 
-- 
2.25.1

[RFC PATCH v2 net-next 2/5] net: phylink: introduce a generic method for querying PHY in-band autoneg capability

[Vladimir Oltean]

Phylink parses the firmware node for 'managed = "in-band-status"' and
populates the initial pl->cfg_link_an_mode to MLO_AN_PHY or MLO_AN_INBAND
accordingly, but sometimes things do not really work out at runtime, and
the pl->cur_link_an_mode may change.

The most notable case is when an SFP module with a PHY that has broken
in-band autoneg is attached. Phylink currently open-codes a check for
the BCM84881 PHY ID and updates pl->cur_link_an_mode from MLO_AN_INBAND
to MLO_AN_PHY.

There is an additional degree of freedom I would like to add. This has
to do with the on-board PHY case (not on SFP). Sometimes, a PHY can only
operate with in-band autoneg enabled, but the MAC driver does not
declare 'managed = "in-band-status"' in the firmware node (say it was
recently converted from phylib to phylink). If the MAC driver is strict
in its phylink ops implementation, it will disable in-band autoneg and
thus the connection to the PHY will be broken.

The firmware can (and should) be updated, but if the PHY driver is
patched to report that it only supports in-band autoneg, then the
pl->cur_link_an_mode can be fixed up to request in-band autoneg from the
MAC driver, even if the firmware node does not. While I do not expect
production systems to rely on this feature, it seems sensible to have it
as long as it is not difficult to implement (the PHY driver should be
updated with a small .validate_inband_aneg method), and it can even ease
the transition from phylib to phylink.

There is also the reverse case: the firmware node reports MLO_AN_INBAND
but the on-board PHY doesn't support that. That sounds like a serious
bug, so while we still do attempt to fix it up (it seems within our
reach to handle it, and worth it), we print to the kernel log on a more
severe tone and not just at the debug level.

So if the 3 code paths:
- phylink_sfp_config
- phylink_connect_phy
- phylink_fwnode_phy_connect

do more or less the same thing (adapt pl->cur_link_an_mode based on the
capability reported by the PHY), the intention is different. With SFP
modules this behavior is absolutely to be expected, and pl->cfg_link_an_mode
only denotes the initial operating mode. On the other hand, when the PHY
is on-board, the initial link AN mode should ideally also be the final
one. So the implementations for the three are different.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/phy.c     | 13 ++++++++
 drivers/net/phy/phylink.c | 63 +++++++++++++++++++++++++++++++++++++--
 include/linux/phy.h       | 16 ++++++++++
 3 files changed, 89 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index f124a8a58bd4..975ae3595f8f 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -750,6 +750,19 @@ static int phy_check_link_status(struct phy_device *phydev)
 	return 0;
 }
 
+int phy_validate_inband_aneg(struct phy_device *phydev,
+			     phy_interface_t interface)
+{
+	if (!phydev->drv)
+		return -EIO;
+
+	if (!phydev->drv->validate_inband_aneg)
+		return PHY_INBAND_ANEG_UNKNOWN;
+
+	return phydev->drv->validate_inband_aneg(phydev, interface);
+}
+EXPORT_SYMBOL_GPL(phy_validate_inband_aneg);
+
 /**
  * phy_start_aneg - start auto-negotiation for this PHY device
  * @phydev: the phy_device struct
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 28edb3665ee9..6bded664ad86 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1042,6 +1042,39 @@ static int phylink_attach_phy(struct phylink *pl, struct phy_device *phy,
 	return phy_attach_direct(pl->netdev, phy, 0, interface);
 }
 
+static unsigned int phylink_fixup_inband_aneg(struct phylink *pl,
+					      struct phy_device *phy,
+					      unsigned int mode)
+{
+	int ret;
+
+	ret = phy_validate_inband_aneg(phy, pl->link_interface);
+	if (ret == PHY_INBAND_ANEG_UNKNOWN) {
+		phylink_dbg(pl,
+			    "PHY driver does not report in-band autoneg capability, assuming %s\n",
+			    phylink_autoneg_inband(mode) ? "true" : "false");
+
+		return mode;
+	}
+
+	if (phylink_autoneg_inband(mode) && !(ret & PHY_INBAND_ANEG_ON)) {
+		phylink_err(pl,
+			    "Requested in-band autoneg but driver does not support this, disabling it.\n");
+
+		return MLO_AN_PHY;
+	}
+
+	if (!phylink_autoneg_inband(mode) && !(ret & PHY_INBAND_ANEG_OFF)) {
+		phylink_dbg(pl,
+			    "PHY driver requests in-band autoneg, force-enabling it.\n");
+
+		mode = MLO_AN_INBAND;
+	}
+
+	/* Peaceful agreement, isn't it great? */
+	return mode;
+}
+
 /**
  * phylink_connect_phy() - connect a PHY to the phylink instance
  * @pl: a pointer to a &struct phylink returned from phylink_create()
@@ -1061,6 +1094,9 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 {
 	int ret;
 
+	pl->cur_link_an_mode = phylink_fixup_inband_aneg(pl, phy,
+							 pl->cfg_link_an_mode);
+
 	/* Use PHY device/driver interface */
 	if (pl->link_interface == PHY_INTERFACE_MODE_NA) {
 		pl->link_interface = phy->interface;
@@ -1136,6 +1172,9 @@ int phylink_fwnode_phy_connect(struct phylink *pl,
 	if (!phy_dev)
 		return -ENODEV;
 
+	pl->cur_link_an_mode = phylink_fixup_inband_aneg(pl, phy_dev,
+							 pl->cfg_link_an_mode);
+
 	ret = phy_attach_direct(pl->netdev, phy_dev, flags,
 				pl->link_interface);
 	if (ret) {
@@ -2096,10 +2135,28 @@ static int phylink_sfp_config(struct phylink *pl, struct phy_device *phy,
 		return -EINVAL;
 	}
 
-	if (phy && phylink_phy_no_inband(phy))
-		mode = MLO_AN_PHY;
-	else
+	/* Select whether to operate in in-band mode or not, based on the
+	 * presence and capability of the PHY in the current link mode.
+	 */
+	if (phy) {
+		ret = phy_validate_inband_aneg(phy, iface);
+		if (ret == PHY_INBAND_ANEG_UNKNOWN) {
+			if (phylink_phy_no_inband(phy))
+				mode = MLO_AN_PHY;
+			else
+				mode = MLO_AN_INBAND;
+
+			phylink_dbg(pl,
+				    "PHY driver does not report in-band autoneg capability, assuming %s\n",
+				    phylink_autoneg_inband(mode) ? "true" : "false");
+		} else if (ret & PHY_INBAND_ANEG_ON) {
+			mode = MLO_AN_INBAND;
+		} else {
+			mode = MLO_AN_PHY;
+		}
+	} else {
 		mode = MLO_AN_INBAND;
+	}
 
 	config.interface = iface;
 	linkmode_copy(support1, support);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 736e1d1a47c4..4ac876f988ca 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -698,6 +698,12 @@ struct phy_tdr_config {
 };
 #define PHY_PAIR_ALL -1
 
+enum phy_inband_aneg {
+	PHY_INBAND_ANEG_UNKNOWN		= BIT(0),
+	PHY_INBAND_ANEG_OFF		= BIT(1),
+	PHY_INBAND_ANEG_ON		= BIT(2),
+};
+
 /**
  * struct phy_driver - Driver structure for a particular PHY type
  *
@@ -767,6 +773,14 @@ struct phy_driver {
 	 */
 	int (*config_aneg)(struct phy_device *phydev);
 
+	/**
+	 * @validate_inband_aneg: Report what types of in-band auto-negotiation
+	 * are available for the given PHY interface type. Returns a bit mask
+	 * of type enum phy_inband_aneg.
+	 */
+	int (*validate_inband_aneg)(struct phy_device *phydev,
+				    phy_interface_t interface);
+
 	/** @aneg_done: Determines the auto negotiation result */
 	int (*aneg_done)(struct phy_device *phydev);
 
@@ -1458,6 +1472,8 @@ void phy_start(struct phy_device *phydev);
 void phy_stop(struct phy_device *phydev);
 int phy_config_aneg(struct phy_device *phydev);
 int phy_start_aneg(struct phy_device *phydev);
+int phy_validate_inband_aneg(struct phy_device *phydev,
+			     phy_interface_t interface);
 int phy_aneg_done(struct phy_device *phydev);
 int phy_speed_down(struct phy_device *phydev, bool sync);
 int phy_speed_up(struct phy_device *phydev);
-- 
2.25.1

[RFC PATCH v2 net-next 3/5] net: phy: bcm84881: move the in-band capability check where it belongs

[Vladimir Oltean]

Now that there is a generic interface through which phylink can query
PHY drivers whether they support various forms of in-band autoneg, use
that and delete the special case from phylink.c.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/bcm84881.c | 10 ++++++++++
 drivers/net/phy/phylink.c  | 17 ++---------------
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/net/phy/bcm84881.c b/drivers/net/phy/bcm84881.c
index 9717a1626f3f..5c0e4f85fc4e 100644
--- a/drivers/net/phy/bcm84881.c
+++ b/drivers/net/phy/bcm84881.c
@@ -223,6 +223,15 @@ static int bcm84881_read_status(struct phy_device *phydev)
 	return genphy_c45_read_mdix(phydev);
 }
 
+/* The Broadcom BCM84881 in the Methode DM7052 is unable to provide a SGMII
+ * or 802.3z control word, so inband will not work.
+ */
+static int bcm84881_validate_inband_aneg(struct phy_device *phydev,
+					 phy_interface_t interface)
+{
+	return PHY_INBAND_ANEG_OFF;
+}
+
 static struct phy_driver bcm84881_drivers[] = {
 	{
 		.phy_id		= 0xae025150,
@@ -234,6 +243,7 @@ static struct phy_driver bcm84881_drivers[] = {
 		.config_aneg	= bcm84881_config_aneg,
 		.aneg_done	= bcm84881_aneg_done,
 		.read_status	= bcm84881_read_status,
+		.validate_inband_aneg = bcm84881_validate_inband_aneg,
 	},
 };
 
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 6bded664ad86..7f4455b74569 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2074,15 +2074,6 @@ int phylink_speed_up(struct phylink *pl)
 }
 EXPORT_SYMBOL_GPL(phylink_speed_up);
 
-/* The Broadcom BCM84881 in the Methode DM7052 is unable to provide a SGMII
- * or 802.3z control word, so inband will not work.
- */
-static bool phylink_phy_no_inband(struct phy_device *phy)
-{
-	return phy->is_c45 &&
-		(phy->c45_ids.device_ids[1] & 0xfffffff0) == 0xae025150;
-}
-
 static void phylink_sfp_attach(void *upstream, struct sfp_bus *bus)
 {
 	struct phylink *pl = upstream;
@@ -2141,14 +2132,10 @@ static int phylink_sfp_config(struct phylink *pl, struct phy_device *phy,
 	if (phy) {
 		ret = phy_validate_inband_aneg(phy, iface);
 		if (ret == PHY_INBAND_ANEG_UNKNOWN) {
-			if (phylink_phy_no_inband(phy))
-				mode = MLO_AN_PHY;
-			else
-				mode = MLO_AN_INBAND;
+			mode = MLO_AN_INBAND;
 
 			phylink_dbg(pl,
-				    "PHY driver does not report in-band autoneg capability, assuming %s\n",
-				    phylink_autoneg_inband(mode) ? "true" : "false");
+				    "PHY driver does not report in-band autoneg capability, assuming true\n");
 		} else if (ret & PHY_INBAND_ANEG_ON) {
 			mode = MLO_AN_INBAND;
 		} else {
-- 
2.25.1

[RFC PATCH v2 net-next 4/5] net: phylink: explicitly configure in-band autoneg for PHYs that support it

[Vladimir Oltean]

Currently Linux has no control over whether a MAC-to-PHY interface uses
in-band signaling or not, even though phylink has the
	managed = "in-band-status";
property which denotes that the MAC expects in-band signaling to be used.

The problem is really that if the in-band signaling is configurable in
both the PHY and the MAC, there is a risk that they are out of sync
unless phylink manages them both. Most if not all in-band autoneg state
machines follow IEEE 802.3 clause 37, which means that they will not
change the operating mode of the SERDES lane from control to data mode
unless in-band AN completed successfully. Therefore traffic will not
work.

It is particularly unpleasant that currently, we assume that PHYs which
have configurable in-band AN come pre-configured from a prior boot stage
such as U-Boot, because once the bootloader changes, all bets are off.

Let's introduce a new PHY driver method for configuring in-band autoneg,
and make phylink be its first user. The main PHY library does not call
phy_config_inband_autoneg, because it does not know what to configure it
to. Presumably, non-phylink drivers can also call phy_config_inband_autoneg
individually.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/phy.c     | 12 ++++++++++++
 drivers/net/phy/phylink.c | 10 ++++++++++
 include/linux/phy.h       |  8 ++++++++
 3 files changed, 30 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 975ae3595f8f..3adc818db30d 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -763,6 +763,18 @@ int phy_validate_inband_aneg(struct phy_device *phydev,
 }
 EXPORT_SYMBOL_GPL(phy_validate_inband_aneg);
 
+int phy_config_inband_aneg(struct phy_device *phydev, bool enabled)
+{
+	if (!phydev->drv)
+		return -EIO;
+
+	if (!phydev->drv->config_inband_aneg)
+		return -EOPNOTSUPP;
+
+	return phydev->drv->config_inband_aneg(phydev, enabled);
+}
+EXPORT_SYMBOL_GPL(phy_config_inband_aneg);
+
 /**
  * phy_start_aneg - start auto-negotiation for this PHY device
  * @phydev: the phy_device struct
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 7f4455b74569..167f91c59e0d 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -954,6 +954,7 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy,
 {
 	struct phylink_link_state config;
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+	bool use_inband;
 	char *irq_str;
 	int ret;
 
@@ -993,6 +994,15 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy,
 		return ret;
 	}
 
+	use_inband = phylink_autoneg_inband(pl->cur_link_an_mode);
+
+	ret = phy_config_inband_aneg(phy, use_inband);
+	if (ret && ret != -EOPNOTSUPP) {
+		phylink_warn(pl, "failed to configure PHY in-band autoneg: %pe\n",
+			     ERR_PTR(ret));
+		return ret;
+	}
+
 	phy->phylink = pl;
 	phy->phy_link_change = phylink_phy_change;
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 4ac876f988ca..c81c6554d564 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -781,6 +781,13 @@ struct phy_driver {
 	int (*validate_inband_aneg)(struct phy_device *phydev,
 				    phy_interface_t interface);
 
+	/**
+	 * @config_inband_aneg: Enable or disable in-band auto-negotiation for
+	 * the system-side interface if the PHY operates in a mode that
+	 * requires it: (Q)SGMII, USXGMII, 1000Base-X, etc.
+	 */
+	int (*config_inband_aneg)(struct phy_device *phydev, bool enabled);
+
 	/** @aneg_done: Determines the auto negotiation result */
 	int (*aneg_done)(struct phy_device *phydev);
 
@@ -1474,6 +1481,7 @@ int phy_config_aneg(struct phy_device *phydev);
 int phy_start_aneg(struct phy_device *phydev);
 int phy_validate_inband_aneg(struct phy_device *phydev,
 			     phy_interface_t interface);
+int phy_config_inband_aneg(struct phy_device *phydev, bool enabled);
 int phy_aneg_done(struct phy_device *phydev);
 int phy_speed_down(struct phy_device *phydev, bool sync);
 int phy_speed_up(struct phy_device *phydev);
-- 
2.25.1

[RFC PATCH v2 net-next 5/5] net: phy: mscc: configure in-band auto-negotiation for VSC8514

[Vladimir Oltean]

Add the in-band configuration knob for the VSC8514 quad PHY. Tested with
QSGMII in-band AN both on and off on NXP LS1028A-RDB and T1040-RDB.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/mscc/mscc.h      |  2 ++
 drivers/net/phy/mscc/mscc_main.c | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/net/phy/mscc/mscc.h b/drivers/net/phy/mscc/mscc.h
index a50235fdf7d9..366db1425561 100644
--- a/drivers/net/phy/mscc/mscc.h
+++ b/drivers/net/phy/mscc/mscc.h
@@ -195,6 +195,8 @@ enum rgmii_clock_delay {
 #define MSCC_PHY_EXTENDED_INT_MS_EGR	  BIT(9)
 
 /* Extended Page 3 Registers */
+#define MSCC_PHY_SERDES_PCS_CTRL	  16
+#define MSCC_PHY_SERDES_ANEG		  BIT(7)
 #define MSCC_PHY_SERDES_TX_VALID_CNT	  21
 #define MSCC_PHY_SERDES_TX_CRC_ERR_CNT	  22
 #define MSCC_PHY_SERDES_RX_VALID_CNT	  28
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 6e32da28e138..ca537204bc27 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -2189,6 +2189,24 @@ static int vsc85xx_read_status(struct phy_device *phydev)
 	return genphy_read_status(phydev);
 }
 
+static int vsc8514_validate_inband_aneg(struct phy_device *phydev,
+					phy_interface_t interface)
+{
+	return PHY_INBAND_ANEG_OFF | PHY_INBAND_ANEG_ON;
+}
+
+static int vsc8514_config_inband_aneg(struct phy_device *phydev, bool enabled)
+{
+	int reg_val = 0;
+
+	if (enabled)
+		reg_val = MSCC_PHY_SERDES_ANEG;
+
+	return phy_modify_paged(phydev, MSCC_PHY_PAGE_EXTENDED_3,
+				MSCC_PHY_SERDES_PCS_CTRL, MSCC_PHY_SERDES_ANEG,
+				reg_val);
+}
+
 static int vsc8514_probe(struct phy_device *phydev)
 {
 	struct vsc8531_private *vsc8531;
@@ -2395,6 +2413,8 @@ static struct phy_driver vsc85xx_driver[] = {
 	.get_sset_count = &vsc85xx_get_sset_count,
 	.get_strings    = &vsc85xx_get_strings,
 	.get_stats      = &vsc85xx_get_stats,
+	.validate_inband_aneg = vsc8514_validate_inband_aneg,
+	.config_inband_aneg = vsc8514_config_inband_aneg,
 },
 {
 	.phy_id		= PHY_ID_VSC8530,
-- 
2.25.1

Re: [RFC PATCH v2 net-next 0/5] Let phylink manage in-band AN for the PHY

[Russell King (Oracle)]

Can we postpone this after this merge window please, so I've got time
to properly review this. Thanks.

On Mon, Aug 30, 2021 at 06:52:45PM +0300, Vladimir Oltean wrote:
> This small series creates a configuration knob for PHY drivers which use
> serial MII-side interfaces and support clause 37 in-band auto-negotiation
> there.
> 
> Changes in v2:
> Incorporated feedback from Russell, which was to consider PHYs on SFP
> modules too, and unify phylink's detection of PHYs with broken in-band
> autoneg with the newly introduced PHY driver methods.
> https://patchwork.kernel.org/project/netdevbpf/cover/20210212172341.3489046-1-olteanv@gmail.com/
> 
> This change set is only superficially tested, hence the RFC tag. It does
> what I need on the NXP boards with on-board PHYs that I have, and also
> seems to behave the same as before when I use a 1G SGMII SFP module with
> the Marvell 88E1111 PHY (the only thing I have). I do not have the
> ability to test the Methode DM7052 SFP module for the bcm84881.c driver
> change, since I don't have that.
> 
> Posting the patch series mostly to figure out whether I understood the
> change request correctly.
> 
> Vladimir Oltean (5):
>   net: phylink: pass the phy argument to phylink_sfp_config
>   net: phylink: introduce a generic method for querying PHY in-band
>     autoneg capability
>   net: phy: bcm84881: move the in-band capability check where it belongs
>   net: phylink: explicitly configure in-band autoneg for PHYs that
>     support it
>   net: phy: mscc: configure in-band auto-negotiation for VSC8514
> 
>  drivers/net/phy/bcm84881.c       | 10 ++++
>  drivers/net/phy/mscc/mscc.h      |  2 +
>  drivers/net/phy/mscc/mscc_main.c | 20 +++++++
>  drivers/net/phy/phy.c            | 25 +++++++++
>  drivers/net/phy/phylink.c        | 93 +++++++++++++++++++++++++-------
>  include/linux/phy.h              | 24 +++++++++
>  6 files changed, 154 insertions(+), 20 deletions(-)
> 
> -- 
> 2.25.1
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [RFC PATCH v2 net-next 0/5] Let phylink manage in-band AN for the PHY

[Vladimir Oltean]

On Mon, Aug 30, 2021 at 07:30:15PM +0100, Russell King (Oracle) wrote:
> Can we postpone this after this merge window please, so I've got time
> to properly review this. Thanks.

Please review at your discretion, I've no intention to post a v3 right
now, and to the best of my knowledge, RFC's are not even considered for
direct inclusion in the git tree.

Re: [RFC PATCH v2 net-next 0/5] Let phylink manage in-band AN for the PHY

[Vladimir Oltean]

On Mon, Aug 30, 2021 at 09:36:23PM +0300, Vladimir Oltean wrote:
> On Mon, Aug 30, 2021 at 07:30:15PM +0100, Russell King (Oracle) wrote:
> > Can we postpone this after this merge window please, so I've got time
> > to properly review this. Thanks.
> 
> Please review at your discretion, I've no intention to post a v3 right
> now, and to the best of my knowledge, RFC's are not even considered for
> direct inclusion in the git tree.

Hello Russell, can you please review these patches if possible? I would like to repost them soon.

Re: [RFC PATCH v2 net-next 0/5] Let phylink manage in-band AN for the PHY

[Michael Walle]

Am 2021-09-16 15:09, schrieb Vladimir Oltean:
> On Mon, Aug 30, 2021 at 09:36:23PM +0300, Vladimir Oltean wrote:
>> On Mon, Aug 30, 2021 at 07:30:15PM +0100, Russell King (Oracle) wrote:
>> > Can we postpone this after this merge window please, so I've got time
>> > to properly review this. Thanks.
>> 
>> Please review at your discretion, I've no intention to post a v3 right
>> now, and to the best of my knowledge, RFC's are not even considered 
>> for
>> direct inclusion in the git tree.
> 
> Hello Russell, can you please review these patches if possible? I
> would like to repost them soon.

I planned to test this on my board with the AR8031 (and add support 
there),
but it seems I won't find time before my vacation, unfortunately.

-michael

Re: [RFC PATCH v2 net-next 0/5] Let phylink manage in-band AN for the PHY

[Vladimir Oltean]

On Thu, Sep 16, 2021 at 03:51:28PM +0200, Michael Walle wrote:
> Am 2021-09-16 15:09, schrieb Vladimir Oltean:
> > On Mon, Aug 30, 2021 at 09:36:23PM +0300, Vladimir Oltean wrote:
> > > On Mon, Aug 30, 2021 at 07:30:15PM +0100, Russell King (Oracle) wrote:
> > > > Can we postpone this after this merge window please, so I've got time
> > > > to properly review this. Thanks.
> > > 
> > > Please review at your discretion, I've no intention to post a v3 right
> > > now, and to the best of my knowledge, RFC's are not even considered
> > > for
> > > direct inclusion in the git tree.
> > 
> > Hello Russell, can you please review these patches if possible? I
> > would like to repost them soon.
> 
> I planned to test this on my board with the AR8031 (and add support there),
> but it seems I won't find time before my vacation, unfortunately.

Oh, but there isn't any "support" to be added I though, your conclusion
last time seemed to be that it only supported in-band autoneg ON?
I was going to add a patch to implement .validate_inband_aneg for the
at803x driver to mark that fact too, I just didn't do it in the RFC.
That should also fix the ENETC ports on the LS1028A-RDB which were
migrated to phylink while they didn't have the 'managed = "in-band-status"'
OF property, and enable new kernels to still work with the old DT blob.
Or were you thinking of something else?

Re: [RFC PATCH v2 net-next 0/5] Let phylink manage in-band AN for the PHY

[Michael Walle]

Am 2021-09-16 15:54, schrieb Vladimir Oltean:
> On Thu, Sep 16, 2021 at 03:51:28PM +0200, Michael Walle wrote:
>> Am 2021-09-16 15:09, schrieb Vladimir Oltean:
>> > On Mon, Aug 30, 2021 at 09:36:23PM +0300, Vladimir Oltean wrote:
>> > > On Mon, Aug 30, 2021 at 07:30:15PM +0100, Russell King (Oracle) wrote:
>> > > > Can we postpone this after this merge window please, so I've got time
>> > > > to properly review this. Thanks.
>> > >
>> > > Please review at your discretion, I've no intention to post a v3 right
>> > > now, and to the best of my knowledge, RFC's are not even considered
>> > > for
>> > > direct inclusion in the git tree.
>> >
>> > Hello Russell, can you please review these patches if possible? I
>> > would like to repost them soon.
>> 
>> I planned to test this on my board with the AR8031 (and add support 
>> there),
>> but it seems I won't find time before my vacation, unfortunately.
> 
> Oh, but there isn't any "support" to be added I though, your conclusion
> last time seemed to be that it only supported in-band autoneg ON?
> I was going to add a patch to implement .validate_inband_aneg for the
> at803x driver to mark that fact too, I just didn't do it in the RFC.
> That should also fix the ENETC ports on the LS1028A-RDB which were
> migrated to phylink while they didn't have the 'managed = 
> "in-band-status"'
> OF property, and enable new kernels to still work with the old DT blob.
> Or were you thinking of something else?

No, but I won't find time to test it within the next.. uhm, 30minutes
until I call it a day ;)

-michael

Re: [RFC PATCH v2 net-next 0/5] Let phylink manage in-band AN for the PHY

[Vladimir Oltean]

On Thu, Sep 16, 2021 at 04:05:20PM +0200, Michael Walle wrote:
> Am 2021-09-16 15:54, schrieb Vladimir Oltean:
> > On Thu, Sep 16, 2021 at 03:51:28PM +0200, Michael Walle wrote:
> > > Am 2021-09-16 15:09, schrieb Vladimir Oltean:
> > > > On Mon, Aug 30, 2021 at 09:36:23PM +0300, Vladimir Oltean wrote:
> > > > > On Mon, Aug 30, 2021 at 07:30:15PM +0100, Russell King (Oracle) wrote:
> > > > > > Can we postpone this after this merge window please, so I've got time
> > > > > > to properly review this. Thanks.
> > > > >
> > > > > Please review at your discretion, I've no intention to post a v3 right
> > > > > now, and to the best of my knowledge, RFC's are not even considered
> > > > > for
> > > > > direct inclusion in the git tree.
> > > >
> > > > Hello Russell, can you please review these patches if possible? I
> > > > would like to repost them soon.
> > > 
> > > I planned to test this on my board with the AR8031 (and add support
> > > there),
> > > but it seems I won't find time before my vacation, unfortunately.
> > 
> > Oh, but there isn't any "support" to be added I though, your conclusion
> > last time seemed to be that it only supported in-band autoneg ON?
> > I was going to add a patch to implement .validate_inband_aneg for the
> > at803x driver to mark that fact too, I just didn't do it in the RFC.
> > That should also fix the ENETC ports on the LS1028A-RDB which were
> > migrated to phylink while they didn't have the 'managed =
> > "in-band-status"'
> > OF property, and enable new kernels to still work with the old DT blob.
> > Or were you thinking of something else?
> 
> No, but I won't find time to test it within the next.. uhm, 30minutes
> until I call it a day ;)

Ok, if that is all, I can make sure on the NXP LS1028A-RDB that the
Atheros PHYs are always presented to phylink drivers as MLO_AN_INBAND,
never MLO_AN_PHY, and make sure that the enetc driver works in that
configuration regardless of device tree description.