Date:   Fri, 18 Nov 2022 02:01:18 +0200
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     netdev@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Russell King <linux@...linux.org.uk>,
        Florian Fainelli <f.fainelli@...il.com>,
        UNGLinuxDriver@...rochip.com,
        bcm-kernel-feedback-list@...adcom.com,
        Madalin Bucur <madalin.bucur@....nxp.com>,
        Camelia Groza <camelia.groza@....com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Maxim Kochetkov <fido_max@...ox.ru>,
        Sean Anderson <sean.anderson@...o.com>,
        Antoine Tenart <atenart@...nel.org>,
        Michael Walle <michael@...le.cc>,
        Raag Jadav <raagjadav@...il.com>,
        Siddharth Vadapalli <s-vadapalli@...com>,
        Ong Boon Leong <boon.leong.ong@...el.com>,
        Colin Foster <colin.foster@...advantage.com>,
        Marek Behun <marek.behun@....cz>
Subject: [PATCH v4 net-next 2/8] net: phylink: introduce generic method to query PHY in-band autoneg capability

Currently, phylink requires that fwnodes with links to SFP cages have
the 'managed = "in-band-status"' property, and based on this, the
initial pl->cfg_link_an_mode gets set to MLO_AN_INBAND.

However, some PHYs on SFP modules may have broken in-band autoneg, and
in that case, phylink selects a pl->cur_link_an_mode which is MLO_AN_PHY,
to tell the MAC/PCS side to disable in-band autoneg (link speed/status
will come over the MDIO side channel).

The check for PHY in-band autoneg capability is currently open-coded
based on a PHY ID comparison against the BCM84881. But the same problem
will also be need to solved in another case, where syncing in-band
autoneg will be desired between the MAC/PCS and an on-board PHY.
So the approach needs to be generalized, and eventually what is done for
the BCM84881 needs to be replaced with a more generic solution.

Add new API to the PHY device structure which allows it to report what
it supports in terms of in-band autoneg (whether it can operate with it
on, and whether it can operate with it off). The assumption is that
there is a Clause 37 compatible state machine in the PHY's PCS, and it
requires that the autoneg process completes before the lane transitions
to data mode. If we have a mismatch between in-band autoneg modes, the
system side link will be broken.

Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
---
v3->v4:
- split the SFP cur_link_an_mode fixup to separate patch (this one)
- s/inband_aneg/an_inband/ to be more in line with phylink terminology
- clearer documentation, added kerneldocs
- don't return -EIO in phy_validate_an_inband(), this breaks with the
  Generic PHY driver because the expected return code is a bit mask, not
  a negative integer

 drivers/net/phy/phy.c     | 25 +++++++++++++++++++++++++
 drivers/net/phy/phylink.c | 20 +++++++++++++++++---
 include/linux/phy.h       | 17 +++++++++++++++++
 3 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index e5b6cb1a77f9..2abbacf2c7cb 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -733,6 +733,31 @@ static int phy_check_link_status(struct phy_device *phydev)
 	return 0;
 }
 
+/**
+ * phy_validate_an_inband - validate which in-band autoneg modes are supported
+ * @phydev: the phy_device struct
+ * @interface: the MAC-side interface type
+ *
+ * Returns @PHY_AN_INBAND_UNKNOWN if it is unknown what in-band autoneg setting
+ * is required for the given PHY mode, or a bit mask of @PHY_AN_INBAND_OFF (if
+ * the PHY is able to work with in-band AN turned off) and @PHY_AN_INBAND_ON
+ * (if it works with the feature turned on). With the Generic PHY driver, the
+ * result will always be @PHY_AN_INBAND_UNKNOWN.
+ */
+int phy_validate_an_inband(struct phy_device *phydev,
+			   phy_interface_t interface)
+{
+	/* We may be called before phy_attach_direct() force-binds the
+	 * generic PHY driver to this device. In that case, report an unknown
+	 * setting rather than -EIO as most other functions do.
+	 */
+	if (!phydev->drv || !phydev->drv->validate_an_inband)
+		return PHY_AN_INBAND_UNKNOWN;
+
+	return phydev->drv->validate_an_inband(phydev, interface);
+}
+EXPORT_SYMBOL_GPL(phy_validate_an_inband);
+
 /**
  * _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 9e4b2dfc98d8..40b7e730fb33 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2936,10 +2936,24 @@ static int phylink_sfp_config_phy(struct phylink *pl, struct phy_device *phy)
 		return -EINVAL;
 	}
 
-	if (phylink_phy_no_inband(phy))
-		mode = MLO_AN_PHY;
-	else
+	/* Select whether to operate in in-band mode or not, based on the
+	 * capability of the PHY in the current link mode.
+	 */
+	ret = phy_validate_an_inband(phy, iface);
+	if (ret == PHY_AN_INBAND_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_AN_INBAND_ON) {
 		mode = MLO_AN_INBAND;
+	} else {
+		mode = MLO_AN_PHY;
+	}
 
 	config.interface = iface;
 	linkmode_copy(support1, support);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 9a3752c0c444..56a431d88dd9 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -761,6 +761,12 @@ struct phy_tdr_config {
 };
 #define PHY_PAIR_ALL -1
 
+enum phy_an_inband {
+	PHY_AN_INBAND_UNKNOWN		= BIT(0),
+	PHY_AN_INBAND_OFF		= BIT(1),
+	PHY_AN_INBAND_ON		= BIT(2),
+};
+
 /**
  * struct phy_driver - Driver structure for a particular PHY type
  *
@@ -845,6 +851,15 @@ struct phy_driver {
 	 */
 	int (*config_aneg)(struct phy_device *phydev);
 
+	/**
+	 * @validate_an_inband: Report what types of in-band auto-negotiation
+	 * are available for the given PHY interface type. Returns a bit mask
+	 * of type enum phy_an_inband. Returning negative error codes is not
+	 * permitted.
+	 */
+	int (*validate_an_inband)(struct phy_device *phydev,
+				  phy_interface_t interface);
+
 	/** @aneg_done: Determines the auto negotiation result */
 	int (*aneg_done)(struct phy_device *phydev);
 
@@ -1540,6 +1555,8 @@ 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_aneg_done(struct phy_device *phydev);
+int phy_validate_an_inband(struct phy_device *phydev,
+			   phy_interface_t interface);
 int phy_speed_down(struct phy_device *phydev, bool sync);
 int phy_speed_up(struct phy_device *phydev);
 
-- 
2.34.1

Powered by blists - more mailing lists