Skip to contentSkip to conversation

net: rework SFP capability parsing and quirks

PATCH Applied Russell King (Oracle) 5 participants 11 messages Sep 16, 2025 – Sep 24, 2025

Subsystems (6)
Networking Network drivers Arm Qualcomm mailing list Ethernet PHY +2 more subsystems
lore
Share & tools
lore

Russell King (Oracle)

author maintainer
#0 lore raw

v2 cover net: rework SFP capability parsing and quirks

The original SPF module parsing was implemented prior to gaining any quirks, and was designed such that the upstream calls the parsing functions to get the translated capabilities of the module.

SFP quirks were then added to cope with modules that didn't correctly fill out their ID EEPROM. The quirk function was called from sfp_parse_support() to allow quirks to modify the ethtool link mode masks.

Using just ethtool link mode masks eventually lead to difficulties determining the correct phy_interface_t mode, so a bitmap of these modes were added - needing both the upstream API and quirks to be updated.

We have had significantly more SFP module quirks added since, some which are modifying the ID EEPROM as a way of influencing the data we provide to the upstream - for example, sfp_fixup_10gbaset_30m() changes id.base.connector so we report PORT_TP. This could be done more cleanly if the quirks had access to the parsed SFP port.

In order to improve flexibility, and to simplify some of the upstream code, we group all module capabilities into a single structure that the upstream can access via sfp_module_get_caps(). This will allow the module capabilities to be expanded if required without reworking all the infrastructure and upstreams again.

In this series, we rework the SFP code to use the capability structure and then rework all the upstream implementations, finally removing the old kernel internal APIs.

---
v2:
- Add Andrew's r-b to patch 1
- sfp_module_may_have_phy() -> sfp_module_parse_may_have_phy()

 drivers/net/phy/marvell-88x2222.c |  13 +++--
 drivers/net/phy/marvell.c         |   8 ++-
 drivers/net/phy/marvell10g.c      |   7 ++-
 drivers/net/phy/phylink.c         |  11 ++--
 drivers/net/phy/qcom/at803x.c     |   9 ++--
 drivers/net/phy/qcom/qca807x.c    |   7 ++-
 drivers/net/phy/sfp-bus.c         | 107 ++++++++++++++++----------------------
 drivers/net/phy/sfp.c             |  49 +++++++++--------
 drivers/net/phy/sfp.h             |   4 +-
 include/linux/phy.h               |   5 ++
 include/linux/sfp.h               |  48 +++++++++--------
 11 files changed, 126 insertions(+), 142 deletions(-)
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#1 lore raw

PATCH 1/7 net: phy: add phy_interface_copy()

Add a helper for copying PHY interface bitmasks. This will be used by the SFP bus code, which will then be moved to phylink in the subsequent patches.

Reviewed-by Andrew Lunn <[email protected]>
Signed-off-by Russell King (Oracle) <[email protected]>
---
 include/linux/phy.h | 5 +++++
 1 file changed, 5 insertions(+)
include/linux/phy.h +5 -0
index 6f3b25cb7f4e..ea7454763516 100644
@@ -169,6 +169,11 @@ static inline bool phy_interface_empty(const unsigned long *intf)
 	return bitmap_empty(intf, PHY_INTERFACE_MODE_MAX);
 }
 
+static inline void phy_interface_copy(unsigned long *d, const unsigned long *s)
+{
+	bitmap_copy(d, s, PHY_INTERFACE_MODE_MAX);
+}
+
 static inline unsigned int phy_interface_weight(const unsigned long *intf)
 {
 	return bitmap_weight(intf, PHY_INTERFACE_MODE_MAX);
Signature · 1 line
2.47.3
#2 lore raw

PATCH 2/7 net: sfp: pre-parse the module support

In reply to Russell King (Oracle)

Pre-parse the module support on insert rather than when the upstream requests the data. This will allow more flexible and extensible parsing.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/phy/sfp-bus.c | 80 +++++++++++++++++++++++++++------------
 include/linux/sfp.h       | 22 +++++++++++
 2 files changed, 77 insertions(+), 25 deletions(-)
drivers/net/phy/sfp-bus.c +55 -25
index f13c00b5b449..35030c527fbe 100644
@@ -22,7 +22,6 @@ struct sfp_bus {
 	const struct sfp_socket_ops *socket_ops;
 	struct device *sfp_dev;
 	struct sfp *sfp;
-	const struct sfp_quirk *sfp_quirk;
 
 	const struct sfp_upstream_ops *upstream_ops;
 	void *upstream;

@@ -30,6 +29,8 @@ struct sfp_bus {
 
 	bool registered;
 	bool started;
+
+	struct sfp_module_caps caps;
 };
 
 /**

@@ -48,6 +49,13 @@ struct sfp_bus {
  */
 int sfp_parse_port(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 		   unsigned long *support)
+{
+	return bus->caps.port;
+}
+EXPORT_SYMBOL_GPL(sfp_parse_port);
+
+static void sfp_module_parse_port(struct sfp_bus *bus,
+				  const struct sfp_eeprom_id *id)
 {
 	int port;
 

@@ -91,21 +99,18 @@ int sfp_parse_port(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 		break;
 	}
 
-	if (support) {
-		switch (port) {
-		case PORT_FIBRE:
-			phylink_set(support, FIBRE);
-			break;
+	switch (port) {
+	case PORT_FIBRE:
+		phylink_set(bus->caps.link_modes, FIBRE);
+		break;
 
-		case PORT_TP:
-			phylink_set(support, TP);
-			break;
-		}
+	case PORT_TP:
+		phylink_set(bus->caps.link_modes, TP);
+		break;
 	}
 
-	return port;
+	bus->caps.port = port;
 }
-EXPORT_SYMBOL_GPL(sfp_parse_port);
 
 /**
  * sfp_may_have_phy() - indicate whether the module may have a PHY

@@ -117,8 +122,17 @@ EXPORT_SYMBOL_GPL(sfp_parse_port);
  */
 bool sfp_may_have_phy(struct sfp_bus *bus, const struct sfp_eeprom_id *id)
 {
-	if (id->base.e1000_base_t)
-		return true;
+	return bus->caps.may_have_phy;
+}
+EXPORT_SYMBOL_GPL(sfp_may_have_phy);
+
+static void sfp_module_parse_may_have_phy(struct sfp_bus *bus,
+					  const struct sfp_eeprom_id *id)
+{
+	if (id->base.e1000_base_t) {
+		bus->caps.may_have_phy = true;
+		return;
+	}
 
 	if (id->base.phys_id != SFF8024_ID_DWDM_SFP) {
 		switch (id->base.extended_cc) {

@@ -126,13 +140,13 @@ bool sfp_may_have_phy(struct sfp_bus *bus, const struct sfp_eeprom_id *id)
 		case SFF8024_ECC_10GBASE_T_SR:
 		case SFF8024_ECC_5GBASE_T:
 		case SFF8024_ECC_2_5GBASE_T:
-			return true;
+			bus->caps.may_have_phy = true;
+			return;
 		}
 	}
 
-	return false;
+	bus->caps.may_have_phy = false;
 }
-EXPORT_SYMBOL_GPL(sfp_may_have_phy);
 
 /**
  * sfp_parse_support() - Parse the eeprom id for supported link modes

@@ -148,8 +162,17 @@ EXPORT_SYMBOL_GPL(sfp_may_have_phy);
 void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 		       unsigned long *support, unsigned long *interfaces)
 {
+	linkmode_or(support, support, bus->caps.link_modes);
+	phy_interface_copy(interfaces, bus->caps.interfaces);
+}
+EXPORT_SYMBOL_GPL(sfp_parse_support);
+
+static void sfp_module_parse_support(struct sfp_bus *bus,
+				     const struct sfp_eeprom_id *id)
+{
+	unsigned long *interfaces = bus->caps.interfaces;
+	unsigned long *modes = bus->caps.link_modes;
 	unsigned int br_min, br_nom, br_max;
-	__ETHTOOL_DECLARE_LINK_MODE_MASK(modes) = { 0, };
 
 	/* Decode the bitrate information to MBd */
 	br_min = br_nom = br_max = 0;

@@ -338,13 +361,22 @@ void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 	phylink_set(modes, Autoneg);
 	phylink_set(modes, Pause);
 	phylink_set(modes, Asym_Pause);
+}
+
+static void sfp_init_module(struct sfp_bus *bus,
+			    const struct sfp_eeprom_id *id,
+			    const struct sfp_quirk *quirk)
+{
+	memset(&bus->caps, 0, sizeof(bus->caps));
 
-	if (bus->sfp_quirk && bus->sfp_quirk->modes)
-		bus->sfp_quirk->modes(id, modes, interfaces);
+	sfp_module_parse_support(bus, id);
+	sfp_module_parse_port(bus, id);
+	sfp_module_parse_may_have_phy(bus, id);
 
-	linkmode_or(support, support, modes);
+	if (quirk && quirk->modes)
+		quirk->modes(id, bus->caps.link_modes,
+			     bus->caps.interfaces);
 }
-EXPORT_SYMBOL_GPL(sfp_parse_support);
 
 /**
  * sfp_select_interface() - Select appropriate phy_interface_t mode

@@ -794,7 +826,7 @@ int sfp_module_insert(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 	const struct sfp_upstream_ops *ops = sfp_get_upstream_ops(bus);
 	int ret = 0;
 
-	bus->sfp_quirk = quirk;
+	sfp_init_module(bus, id, quirk);
 
 	if (ops && ops->module_insert)
 		ret = ops->module_insert(bus->upstream, id);

@@ -809,8 +841,6 @@ void sfp_module_remove(struct sfp_bus *bus)
 
 	if (ops && ops->module_remove)
 		ops->module_remove(bus->upstream);
-
-	bus->sfp_quirk = NULL;
 }
 EXPORT_SYMBOL_GPL(sfp_module_remove);
include/linux/sfp.h +22 -0
index 60c65cea74f6..5fb59cf49882 100644
@@ -521,6 +521,28 @@ struct ethtool_eeprom;
 struct ethtool_modinfo;
 struct sfp_bus;
 
+/**
+ * struct sfp_module_caps - sfp module capabilities
+ * @interfaces: bitmap of interfaces that the module may support
+ * @link_modes: bitmap of ethtool link modes that the module may support
+ */
+struct sfp_module_caps {
+	DECLARE_PHY_INTERFACE_MASK(interfaces);
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(link_modes);
+	/**
+	 * @may_have_phy: indicate whether the module may have an ethernet PHY
+	 * There is no way to be sure that a module has a PHY as the EEPROM
+	 * doesn't contain this information. When set, this does not mean that
+	 * the module definitely has a PHY.
+	 */
+	bool may_have_phy;
+	/**
+	 * @port: one of ethtool %PORT_* definitions, parsed from the module
+	 * EEPROM, or %PORT_OTHER if the port type is not known.
+	 */
+	u8 port;
+};
+
 /**
  * struct sfp_upstream_ops - upstream operations structure
  * @attach: called when the sfp socket driver is bound to the upstream
Signature · 1 line
2.47.3
#3 lore raw

PATCH 3/7 net: sfp: convert sfp quirks to modify struct sfp_module_support

In reply to Russell King (Oracle)

In order to provide extensible module support properties, arrange for the SFP quirks to modify any member of the sfp_module_support struct, rather than just the ethtool link modes and interfaces.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/phy/sfp-bus.c |  5 ++--
 drivers/net/phy/sfp.c     | 49 +++++++++++++++++++--------------------
 drivers/net/phy/sfp.h     |  4 ++--
 3 files changed, 28 insertions(+), 30 deletions(-)
drivers/net/phy/sfp-bus.c +2 -3
index 35030c527fbe..b77190494b04 100644
@@ -373,9 +373,8 @@ static void sfp_init_module(struct sfp_bus *bus,
 	sfp_module_parse_port(bus, id);
 	sfp_module_parse_may_have_phy(bus, id);
 
-	if (quirk && quirk->modes)
-		quirk->modes(id, bus->caps.link_modes,
-			     bus->caps.interfaces);
+	if (quirk && quirk->support)
+		quirk->support(id, &bus->caps);
 }
 
 /**
drivers/net/phy/sfp.c +24 -25

94 lines not on this page · raw message

drivers/net/phy/sfp.h +2 -2
index 1fd097dccb9f..879dff7afe6a 100644
@@ -9,8 +9,8 @@ struct sfp;
 struct sfp_quirk {
 	const char *vendor;
 	const char *part;
-	void (*modes)(const struct sfp_eeprom_id *id, unsigned long *modes,
-		      unsigned long *interfaces);
+	void (*support)(const struct sfp_eeprom_id *id,
+			struct sfp_module_caps *caps);
 	void (*fixup)(struct sfp *sfp);
 };
Signature · 1 line
2.47.3
#4 lore raw

PATCH 4/7 net: sfp: provide sfp_get_module_caps()

In reply to Russell King (Oracle)

Provide a function to retrieve the current sfp_module_caps structure so that upstreams can get the entire module support in one go.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/phy/sfp-bus.c | 6 ++++++
 include/linux/sfp.h       | 7 +++++++
 2 files changed, 13 insertions(+)
drivers/net/phy/sfp-bus.c +6 -0
index b77190494b04..e8cf411396fa 100644
@@ -33,6 +33,12 @@ struct sfp_bus {
 	struct sfp_module_caps caps;
 };
 
+const struct sfp_module_caps *sfp_get_module_caps(struct sfp_bus *bus)
+{
+	return &bus->caps;
+}
+EXPORT_SYMBOL_GPL(sfp_get_module_caps);
+
 /**
  * sfp_parse_port() - Parse the EEPROM base ID, setting the port type
  * @bus: a pointer to the &struct sfp_bus structure for the sfp module
include/linux/sfp.h +7 -0
index 5fb59cf49882..9f29fcad52be 100644
@@ -576,6 +576,7 @@ struct sfp_upstream_ops {
 };
 
 #if IS_ENABLED(CONFIG_SFP)
+const struct sfp_module_caps *sfp_get_module_caps(struct sfp_bus *bus);
 int sfp_parse_port(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
 		   unsigned long *support);
 bool sfp_may_have_phy(struct sfp_bus *bus, const struct sfp_eeprom_id *id);

@@ -600,6 +601,12 @@ int sfp_bus_add_upstream(struct sfp_bus *bus, void *upstream,
 void sfp_bus_del_upstream(struct sfp_bus *bus);
 const char *sfp_get_name(struct sfp_bus *bus);
 #else
+static inline const struct sfp_module_caps *
+sfp_get_module_caps(struct sfp_bus *bus)
+{
+	return NULL;
+}
+
 static inline int sfp_parse_port(struct sfp_bus *bus,
 				 const struct sfp_eeprom_id *id,
 				 unsigned long *support)
Signature · 1 line
2.47.3
#5 lore raw

PATCH 5/7 net: phylink: use sfp_get_module_caps()

In reply to Russell King (Oracle)

Use sfp_get_module_caps() to get SFP module's capabilities.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/phy/phylink.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/net/phy/phylink.c +6 -5
index 1b06805f1bd7..9d7799ea1c17 100644
@@ -3750,17 +3750,18 @@ static int phylink_sfp_config_optical(struct phylink *pl)
 static int phylink_sfp_module_insert(void *upstream,
 				     const struct sfp_eeprom_id *id)
 {
+	const struct sfp_module_caps *caps;
 	struct phylink *pl = upstream;
 
 	ASSERT_RTNL();
 
-	linkmode_zero(pl->sfp_support);
-	phy_interface_zero(pl->sfp_interfaces);
-	sfp_parse_support(pl->sfp_bus, id, pl->sfp_support, pl->sfp_interfaces);
-	pl->sfp_port = sfp_parse_port(pl->sfp_bus, id, pl->sfp_support);
+	caps = sfp_get_module_caps(pl->sfp_bus);
+	phy_interface_copy(pl->sfp_interfaces, caps->interfaces);
+	linkmode_copy(pl->sfp_support, caps->link_modes);
+	pl->sfp_may_have_phy = caps->may_have_phy;
+	pl->sfp_port = caps->port;
 
 	/* If this module may have a PHY connecting later, defer until later */
-	pl->sfp_may_have_phy = sfp_may_have_phy(pl->sfp_bus, id);
 	if (pl->sfp_may_have_phy)
 		return 0;
Signature · 1 line
2.47.3
#6 lore raw

PATCH 6/7 net: phy: update all PHYs to use sfp_get_module_caps()

In reply to Russell King (Oracle)

Update all PHYs to use sfp_get_module_caps() rather than the sfp_parse_*() family of functions.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/phy/marvell-88x2222.c | 13 ++++++-------
 drivers/net/phy/marvell.c         |  8 +++-----
 drivers/net/phy/marvell10g.c      |  7 +++----
 drivers/net/phy/qcom/at803x.c     |  9 ++++-----
 drivers/net/phy/qcom/qca807x.c    |  7 +++----
 5 files changed, 19 insertions(+), 25 deletions(-)
drivers/net/phy/marvell-88x2222.c +6 -7
index fad2f54c1eac..894bcee61e65 100644
@@ -475,21 +475,20 @@ static int mv2222_config_init(struct phy_device *phydev)
 
 static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
 {
-	DECLARE_PHY_INTERFACE_MASK(interfaces);
 	struct phy_device *phydev = upstream;
+	const struct sfp_module_caps *caps;
 	phy_interface_t sfp_interface;
 	struct mv2222_data *priv;
 	struct device *dev;
 	int ret;
 
-	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_supported) = { 0, };
-
 	priv = phydev->priv;
 	dev = &phydev->mdio.dev;
 
-	sfp_parse_support(phydev->sfp_bus, id, sfp_supported, interfaces);
-	phydev->port = sfp_parse_port(phydev->sfp_bus, id, sfp_supported);
-	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
+	caps = sfp_get_module_caps(phydev->sfp_bus);
+
+	phydev->port = caps->port;
+	sfp_interface = sfp_select_interface(phydev->sfp_bus, caps->link_modes);
 
 	dev_info(dev, "%s SFP module inserted\n", phy_modes(sfp_interface));
 

@@ -502,7 +501,7 @@ static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
 	}
 
 	priv->line_interface = sfp_interface;
-	linkmode_and(priv->supported, phydev->supported, sfp_supported);
+	linkmode_and(priv->supported, phydev->supported, caps->link_modes);
 
 	ret = mv2222_config_line(phydev);
 	if (ret < 0)
drivers/net/phy/marvell.c +3 -5
index 0ea366c1217e..c248c90510ae 100644
@@ -3600,20 +3600,18 @@ static int marvell_probe(struct phy_device *phydev)
 
 static int m88e1510_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
 {
-	DECLARE_PHY_INTERFACE_MASK(interfaces);
 	struct phy_device *phydev = upstream;
+	const struct sfp_module_caps *caps;
 	phy_interface_t interface;
 	struct device *dev;
 	int oldpage;
 	int ret = 0;
 	u16 mode;
 
-	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
-
 	dev = &phydev->mdio.dev;
 
-	sfp_parse_support(phydev->sfp_bus, id, supported, interfaces);
-	interface = sfp_select_interface(phydev->sfp_bus, supported);
+	caps = sfp_get_module_caps(phydev->sfp_bus);
+	interface = sfp_select_interface(phydev->sfp_bus, caps->link_modes);
 
 	dev_info(dev, "%s SFP module inserted\n", phy_modes(interface));
drivers/net/phy/marvell10g.c +3 -4
index 13e81dff42c1..8fd42131cdbf 100644
@@ -466,12 +466,11 @@ static int mv3310_set_edpd(struct phy_device *phydev, u16 edpd)
 static int mv3310_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
 {
 	struct phy_device *phydev = upstream;
-	__ETHTOOL_DECLARE_LINK_MODE_MASK(support) = { 0, };
-	DECLARE_PHY_INTERFACE_MASK(interfaces);
+	const struct sfp_module_caps *caps;
 	phy_interface_t iface;
 
-	sfp_parse_support(phydev->sfp_bus, id, support, interfaces);
-	iface = sfp_select_interface(phydev->sfp_bus, support);
+	caps = sfp_get_module_caps(phydev->sfp_bus);
+	iface = sfp_select_interface(phydev->sfp_bus, caps->link_modes);
 
 	if (iface != PHY_INTERFACE_MODE_10GBASER) {
 		dev_err(&phydev->mdio.dev, "incompatible SFP module inserted\n");
drivers/net/phy/qcom/at803x.c +4 -5
index 51a132242462..338acd11a9b6 100644
@@ -771,10 +771,10 @@ static int at8031_register_regulators(struct phy_device *phydev)
 
 static int at8031_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
 {
-	struct phy_device *phydev = upstream;
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(phy_support);
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_support);
-	DECLARE_PHY_INTERFACE_MASK(interfaces);
+	struct phy_device *phydev = upstream;
+	const struct sfp_module_caps *caps;
 	phy_interface_t iface;
 
 	linkmode_zero(phy_support);

@@ -784,12 +784,11 @@ static int at8031_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
 	phylink_set(phy_support, Pause);
 	phylink_set(phy_support, Asym_Pause);
 
-	linkmode_zero(sfp_support);
-	sfp_parse_support(phydev->sfp_bus, id, sfp_support, interfaces);
+	caps = sfp_get_module_caps(phydev->sfp_bus);
 	/* Some modules support 10G modes as well as others we support.
 	 * Mask out non-supported modes so the correct interface is picked.
 	 */
-	linkmode_and(sfp_support, phy_support, sfp_support);
+	linkmode_and(sfp_support, phy_support, caps->link_modes);
 
 	if (linkmode_empty(sfp_support)) {
 		dev_err(&phydev->mdio.dev, "incompatible SFP module inserted\n");
drivers/net/phy/qcom/qca807x.c +3 -4
index 070dc8c00835..1be8295a95cb 100644
@@ -646,13 +646,12 @@ static int qca807x_phy_package_config_init_once(struct phy_device *phydev)
 static int qca807x_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
 {
 	struct phy_device *phydev = upstream;
-	__ETHTOOL_DECLARE_LINK_MODE_MASK(support) = { 0, };
+	const struct sfp_module_caps *caps;
 	phy_interface_t iface;
 	int ret;
-	DECLARE_PHY_INTERFACE_MASK(interfaces);
 
-	sfp_parse_support(phydev->sfp_bus, id, support, interfaces);
-	iface = sfp_select_interface(phydev->sfp_bus, support);
+	caps = sfp_get_module_caps(phydev->sfp_bus);
+	iface = sfp_select_interface(phydev->sfp_bus, caps->link_modes);
 
 	dev_info(&phydev->mdio.dev, "%s SFP module inserted\n", phy_modes(iface));
Signature · 1 line
2.47.3
#7 lore raw

PATCH 7/7 net: sfp: remove old sfp_parse_* functions

In reply to Russell King (Oracle)

Remove the old sfp_parse_*() functions that are now no longer used.

Signed-off-by Russell King (Oracle) <[email protected]>
---
 drivers/net/phy/sfp-bus.c | 54 ---------------------------------------
 include/linux/sfp.h       | 25 ------------------
 2 files changed, 79 deletions(-)
drivers/net/phy/sfp-bus.c +0 -54
index e8cf411396fa..b945d75966d5 100644
@@ -39,27 +39,6 @@ const struct sfp_module_caps *sfp_get_module_caps(struct sfp_bus *bus)
 }
 EXPORT_SYMBOL_GPL(sfp_get_module_caps);
 
-/**
- * sfp_parse_port() - Parse the EEPROM base ID, setting the port type
- * @bus: a pointer to the &struct sfp_bus structure for the sfp module
- * @id: a pointer to the module's &struct sfp_eeprom_id
- * @support: optional pointer to an array of unsigned long for the
- *   ethtool support mask
- *
- * Parse the EEPROM identification given in @id, and return one of
- * %PORT_TP, %PORT_FIBRE or %PORT_OTHER. If @support is non-%NULL,
- * also set the ethtool %ETHTOOL_LINK_MODE_xxx_BIT corresponding with
- * the connector type.
- *
- * If the port type is not known, returns %PORT_OTHER.
- */
-int sfp_parse_port(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
-		   unsigned long *support)
-{
-	return bus->caps.port;
-}
-EXPORT_SYMBOL_GPL(sfp_parse_port);
-
 static void sfp_module_parse_port(struct sfp_bus *bus,
 				  const struct sfp_eeprom_id *id)
 {

@@ -118,20 +97,6 @@ static void sfp_module_parse_port(struct sfp_bus *bus,
 	bus->caps.port = port;
 }
 
-/**
- * sfp_may_have_phy() - indicate whether the module may have a PHY
- * @bus: a pointer to the &struct sfp_bus structure for the sfp module
- * @id: a pointer to the module's &struct sfp_eeprom_id
- *
- * Parse the EEPROM identification given in @id, and return whether
- * this module may have a PHY.
- */
-bool sfp_may_have_phy(struct sfp_bus *bus, const struct sfp_eeprom_id *id)
-{
-	return bus->caps.may_have_phy;
-}
-EXPORT_SYMBOL_GPL(sfp_may_have_phy);
-
 static void sfp_module_parse_may_have_phy(struct sfp_bus *bus,
 					  const struct sfp_eeprom_id *id)
 {

@@ -154,25 +119,6 @@ static void sfp_module_parse_may_have_phy(struct sfp_bus *bus,
 	bus->caps.may_have_phy = false;
 }
 
-/**
- * sfp_parse_support() - Parse the eeprom id for supported link modes
- * @bus: a pointer to the &struct sfp_bus structure for the sfp module
- * @id: a pointer to the module's &struct sfp_eeprom_id
- * @support: pointer to an array of unsigned long for the ethtool support mask
- * @interfaces: pointer to an array of unsigned long for phy interface modes
- *		mask
- *
- * Parse the EEPROM identification information and derive the supported
- * ethtool link modes for the module.
- */
-void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
-		       unsigned long *support, unsigned long *interfaces)
-{
-	linkmode_or(support, support, bus->caps.link_modes);
-	phy_interface_copy(interfaces, bus->caps.interfaces);
-}
-EXPORT_SYMBOL_GPL(sfp_parse_support);
-
 static void sfp_module_parse_support(struct sfp_bus *bus,
 				     const struct sfp_eeprom_id *id)
 {
include/linux/sfp.h +0 -25
index 9f29fcad52be..5c71945a5e4d 100644
@@ -577,11 +577,6 @@ struct sfp_upstream_ops {
 
 #if IS_ENABLED(CONFIG_SFP)
 const struct sfp_module_caps *sfp_get_module_caps(struct sfp_bus *bus);
-int sfp_parse_port(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
-		   unsigned long *support);
-bool sfp_may_have_phy(struct sfp_bus *bus, const struct sfp_eeprom_id *id);
-void sfp_parse_support(struct sfp_bus *bus, const struct sfp_eeprom_id *id,
-		       unsigned long *support, unsigned long *interfaces);
 phy_interface_t sfp_select_interface(struct sfp_bus *bus,
 				     const unsigned long *link_modes);
 

@@ -607,26 +602,6 @@ sfp_get_module_caps(struct sfp_bus *bus)
 	return NULL;
 }
 
-static inline int sfp_parse_port(struct sfp_bus *bus,
-				 const struct sfp_eeprom_id *id,
-				 unsigned long *support)
-{
-	return PORT_OTHER;
-}
-
-static inline bool sfp_may_have_phy(struct sfp_bus *bus,
-				    const struct sfp_eeprom_id *id)
-{
-	return false;
-}
-
-static inline void sfp_parse_support(struct sfp_bus *bus,
-				     const struct sfp_eeprom_id *id,
-				     unsigned long *support,
-				     unsigned long *interfaces)
-{
-}
-
 static inline phy_interface_t sfp_select_interface(struct sfp_bus *bus,
 						const unsigned long *link_modes)
 {
Signature · 1 line
2.47.3
maintainer
#8 lore raw

Re: [PATCH net-next 7/7] net: sfp: remove old sfp_parse_* functions

Quoted 3 lines · Russell King (Oracle)

Remove the old sfp_parse_*() functions that are now no longer used.

Signed-off-by Russell King (Oracle) <[email protected]>

Very nice cleanup series! LGTM! Waiting a little more for any comments from Andrew.

Just in case a repost would be needed, please consider:

sed -e 's/sfp_parse_*() functions/sfp_parse_*() and sfp_may_have_phy() functions/'

in the commit message.

thanks,

Paolo

CI Applied
#9 lore raw
In reply to Russell King (Oracle)
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 10 lines · On Tue, 16 Sep 2025 22:46:08 +0100 you wrote:

The original SPF module parsing was implemented prior to gaining any quirks, and was designed such that the upstream calls the parsing functions to get the translated capabilities of the module.

SFP quirks were then added to cope with modules that didn't correctly fill out their ID EEPROM. The quirk function was called from sfp_parse_support() to allow quirks to modify the ethtool link mode masks.

[...]

Here is the summary with links:
- [net-next,1/7] net: phy: add phy_interface_copy()

    https://git.kernel.org/netdev/net-next/c/a571f08d3db2
  - [net-next,2/7] net: sfp: pre-parse the module support
    https://git.kernel.org/netdev/net-next/c/ddae6127afbb
  - [net-next,3/7] net: sfp: convert sfp quirks to modify struct sfp_module_support
    https://git.kernel.org/netdev/net-next/c/a7dc35a9e49b
  - [net-next,4/7] net: sfp: provide sfp_get_module_caps()
    https://git.kernel.org/netdev/net-next/c/64fb4a3ae8a5
  - [net-next,5/7] net: phylink: use sfp_get_module_caps()
    https://git.kernel.org/netdev/net-next/c/cab116519540
  - [net-next,6/7] net: phy: update all PHYs to use sfp_get_module_caps()
    https://git.kernel.org/netdev/net-next/c/4b6276550f07
  - [net-next,7/7] net: sfp: remove old sfp_parse_* functions
    https://git.kernel.org/netdev/net-next/c/9ce138735efc

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
#10 lore raw
In reply to Russell King (Oracle)

Hi Russell,

Quoted 26 lines · Russell King (Oracle)

The original SPF module parsing was implemented prior to gaining any quirks, and was designed such that the upstream calls the parsing functions to get the translated capabilities of the module.

SFP quirks were then added to cope with modules that didn't correctly fill out their ID EEPROM. The quirk function was called from sfp_parse_support() to allow quirks to modify the ethtool link mode masks.

Using just ethtool link mode masks eventually lead to difficulties determining the correct phy_interface_t mode, so a bitmap of these modes were added - needing both the upstream API and quirks to be updated.

We have had significantly more SFP module quirks added since, some which are modifying the ID EEPROM as a way of influencing the data we provide to the upstream - for example, sfp_fixup_10gbaset_30m() changes id.base.connector so we report PORT_TP. This could be done more cleanly if the quirks had access to the parsed SFP port.

In order to improve flexibility, and to simplify some of the upstream code, we group all module capabilities into a single structure that the upstream can access via sfp_module_get_caps(). This will allow the module capabilities to be expanded if required without reworking all the infrastructure and upstreams again.

Quoted 4 lines

In this series, we rework the SFP code to use the capability structure and then rework all the upstream implementations, finally removing the old kernel internal APIs.

That's some nice work !

I however would have like to be in CC for that, and to have gotten the information that you were working on that.

You commented on the phy_port V10 that we were moving away from sfp_select_interface() and at the time, I asked if the approach I was considering for phy_port was correct or not [1].

Without any reply from you I move on and implemented some reworks in the phy_port series, without comments from you on V11/12/13, only to find out now that this whole thing I've added since v10 to come-up with generic interface selection on PHY driver SFP is superseded by that work.

While your approach is definitely better, I'd have appreciated a heads-up that you were working on that, or even be in CC: for that work, as I'm having very limited availabilities to parse the full netdev list :(

In any case, this will make the phy_port series simpler, so thanks for that work

[1] : https://lore.kernel.org/netdev/[email protected]/

Maxime

Quoted 1 line

---