diff mbox series

[net-next,10/10] net: phylink: use the PHY's possible_interfaces if populated

Message ID E1r6VIV-00DDMF-Pi@rmk-PC.armlinux.org.uk (mailing list archive)
State Accepted
Commit 7a1f9a17ee99a3c27577465ce0f6c5f56cf1aacf
Delegated to: Netdev Maintainers
Headers show
Series net: phylink: improve PHY validation | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/codegen success Generated files up to date
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1115 this patch: 1115
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 1142 this patch: 1142
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1142 this patch: 1142
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 85 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Russell King (Oracle) Nov. 24, 2023, 12:28 p.m. UTC 
Some PHYs such as Aquantia, Broadcom 84881, and Marvell 88X33x0 can
switch between a set of interface types depending on the negotiated
media speed, or can use rate adaption for some or all of these
interface types.

We currently assume that these are Clause 45 PHYs that are configured
not to use a specific set of interface modes, which has worked so far,
but is just a work-around. In this workaround, we validate using all
interfaces that the MAC supports, which can lead to extra modes being
advertised that can not be supported.

To properly address this, switch to using the newly introduced PHY
possible_interfaces bitmap which indicates which interface modes will
be used by the PHY as configured. We calculate the union of the PHY's
possible interfaces and MACs supported interfaces, checking that is
non-empty. If the PHY is on a SFP, we further reduce the set by those
which can be used on a SFP module, again checking that is non-empty.
Finally, we validate the subset of interfaces, taking account of
whether rate matching will be used for each individual interface mode.

This becomes independent of whether the PHY is clause 22 or clause 45.

It is encouraged that all PHYs that switch interface modes or use
rate matching should populate phydev->possible_interfaces.

Tested-by: Luo Jie <quic_luoj@quicinc.com>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 67 +++++++++++++++++++++++++++++++--------
 1 file changed, 54 insertions(+), 13 deletions(-)

Comments

Andrew Lunn Nov. 25, 2023, 5:18 p.m. UTC | #1 
On Fri, Nov 24, 2023 at 12:28:39PM +0000, Russell King (Oracle) wrote:
> Some PHYs such as Aquantia, Broadcom 84881, and Marvell 88X33x0 can
> switch between a set of interface types depending on the negotiated
> media speed, or can use rate adaption for some or all of these
> interface types.
> 
> We currently assume that these are Clause 45 PHYs that are configured
> not to use a specific set of interface modes, which has worked so far,
> but is just a work-around. In this workaround, we validate using all
> interfaces that the MAC supports, which can lead to extra modes being
> advertised that can not be supported.
> 
> To properly address this, switch to using the newly introduced PHY
> possible_interfaces bitmap which indicates which interface modes will
> be used by the PHY as configured. We calculate the union of the PHY's
> possible interfaces and MACs supported interfaces, checking that is
> non-empty. If the PHY is on a SFP, we further reduce the set by those
> which can be used on a SFP module, again checking that is non-empty.
> Finally, we validate the subset of interfaces, taking account of
> whether rate matching will be used for each individual interface mode.
> 
> This becomes independent of whether the PHY is clause 22 or clause 45.
> 
> It is encouraged that all PHYs that switch interface modes or use
> rate matching should populate phydev->possible_interfaces.
> 
> Tested-by: Luo Jie <quic_luoj@quicinc.com>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
diff mbox series

Patch

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 39d85e47422e..48d3bd3e9fc7 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -121,6 +121,19 @@  do {									\
 })
 #endif
 
+static const phy_interface_t phylink_sfp_interface_preference[] = {
+	PHY_INTERFACE_MODE_25GBASER,
+	PHY_INTERFACE_MODE_USXGMII,
+	PHY_INTERFACE_MODE_10GBASER,
+	PHY_INTERFACE_MODE_5GBASER,
+	PHY_INTERFACE_MODE_2500BASEX,
+	PHY_INTERFACE_MODE_SGMII,
+	PHY_INTERFACE_MODE_1000BASEX,
+	PHY_INTERFACE_MODE_100BASEX,
+};
+
+static DECLARE_PHY_INTERFACE_MASK(phylink_sfp_interfaces);
+
 /**
  * phylink_set_port_modes() - set the port type modes in the ethtool mask
  * @mask: ethtool link mode mask
@@ -1764,6 +1777,47 @@  static int phylink_validate_phy(struct phylink *pl, struct phy_device *phy,
 				unsigned long *supported,
 				struct phylink_link_state *state)
 {
+	DECLARE_PHY_INTERFACE_MASK(interfaces);
+
+	/* If the PHY provides a bitmap of the interfaces it will be using
+	 * depending on the negotiated media speeds, use this to validate
+	 * which ethtool link modes can be used.
+	 */
+	if (!phy_interface_empty(phy->possible_interfaces)) {
+		/* We only care about the union of the PHY's interfaces and
+		 * those which the host supports.
+		 */
+		phy_interface_and(interfaces, phy->possible_interfaces,
+				  pl->config->supported_interfaces);
+
+		if (phy_interface_empty(interfaces)) {
+			phylink_err(pl, "PHY has no common interfaces\n");
+			return -EINVAL;
+		}
+
+		if (phy_on_sfp(phy)) {
+			/* If the PHY is on a SFP, limit the interfaces to
+			 * those that can be used with a SFP module.
+			 */
+			phy_interface_and(interfaces, interfaces,
+					  phylink_sfp_interfaces);
+
+			if (phy_interface_empty(interfaces)) {
+				phylink_err(pl, "SFP PHY's possible interfaces becomes empty\n");
+				return -EINVAL;
+			}
+		}
+
+		phylink_dbg(pl, "PHY %s uses interfaces %*pbl, validating %*pbl\n",
+			    phydev_name(phy),
+			    (int)PHY_INTERFACE_MODE_MAX,
+			    phy->possible_interfaces,
+			    (int)PHY_INTERFACE_MODE_MAX, interfaces);
+
+		return phylink_validate_mask(pl, phy, supported, state,
+					     interfaces);
+	}
+
 	/* Check whether we would use rate matching for the proposed interface
 	 * mode.
 	 */
@@ -3032,19 +3086,6 @@  static void phylink_sfp_detach(void *upstream, struct sfp_bus *bus)
 	pl->netdev->sfp_bus = NULL;
 }
 
-static const phy_interface_t phylink_sfp_interface_preference[] = {
-	PHY_INTERFACE_MODE_25GBASER,
-	PHY_INTERFACE_MODE_USXGMII,
-	PHY_INTERFACE_MODE_10GBASER,
-	PHY_INTERFACE_MODE_5GBASER,
-	PHY_INTERFACE_MODE_2500BASEX,
-	PHY_INTERFACE_MODE_SGMII,
-	PHY_INTERFACE_MODE_1000BASEX,
-	PHY_INTERFACE_MODE_100BASEX,
-};
-
-static DECLARE_PHY_INTERFACE_MASK(phylink_sfp_interfaces);
-
 static phy_interface_t phylink_choose_sfp_interface(struct phylink *pl,
 						    const unsigned long *intf)
 {