netdev - [RFC PATCH v3 net-next 1/6] net: phylink: pass the phy argument to phylink_sfp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20210922181446.2677089-2-vladimir.oltean@nxp.com>
Date:   Wed, 22 Sep 2021 21:14:41 +0300
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     netdev@...r.kernel.org
Cc:     Antoine Tenart <atenart@...nel.org>,
        Michael Walle <michael@...le.cc>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Maxim Kochetkov <fido_max@...ox.ru>,
        Bjarni Jonasson <bjarni.jonasson@...rochip.com>,
        Steen Hegelund <steen.hegelund@...rochip.com>,
        UNGLinuxDriver@...rochip.com,
        bcm-kernel-feedback-list@...adcom.com,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Claudiu Beznea <claudiu.beznea@...rochip.com>
Subject: [RFC PATCH v3 net-next 1/6] net: phylink: pass the phy argument to phylink_sfp_config

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@....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 5a58c77d0002..fd02ec265a39 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2146,6 +2146,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;
@@ -2160,7 +2169,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)
 {
@@ -2168,6 +2177,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;
 
@@ -2197,6 +2207,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);
@@ -2261,7 +2276,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)
@@ -2280,8 +2295,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)
@@ -2312,20 +2326,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;
 
 	/*
@@ -2337,13 +2341,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