Merge branch 'net-phylink-improve-phy-validation'

Russell King says:

====================
net: phylink: improve PHY validation

One of the issues which has concerned me about the rate matching
implenentation that we have is that phy_get_rate_matching() returns
whether rate matching will be used for a particular interface, and we
enquire only for one interface.

Aquantia PHYs can be programmed with the rate matching and interface
mode settings on a per-media speed basis using the per-speed vendor 1
global configuration registers.

Thus, it is possible for the PHY to be configured to use rate matching
for 10G, 5G, 2.5G with 10GBASE-R, and then SGMII for the remaining
speeds. Therefore, it clearly doesn't make sense to enquire about rate
matching for just one interface mode.

Also, PHYs that change their interfaces are handled sub-optimally, in
that we validate all the interface modes that the host supports, rather
than the interface modes that the PHY will use.

This patch series changes the way we validate PHYs, but in order to do
so, we need to know exactly which interface modes will be used by the
PHY. So that phylib can convey this information, we add
"possible_interfaces" to struct phy_device.

possible_interfaces is to be filled in by a phylib driver once the PHY
is configured (in other words in the PHYs .config_init method) with the
interface modes that it will switch between. This then allows users of
phylib to know which interface modes will be used by the PHY.

This allows us to solve both these issues: where possible_interfaces is
provided, we can validate which ethtool link modes can be supported by
looking at which interface modes that both the PHY and host support,
and request rate matching information for each mode.

This should improve the accuracy of the validation.

Sending this out again without RFC as Jie Luo will need it for the
QCA8084 changes. No changes except to add the attributations already
received. Thanks!
====================

Link: https://lore.kernel.org/r/[e-mail]
Signed-off-by: Jakub Kicinski <[e-mail]>

drivers/net/phy/aquantia/aquantia.h

···
 #define VEND1_GLOBAL_CFG_5G			0x031e
 #define VEND1_GLOBAL_CFG_10G			0x031f
 /* ...and now the fields */
+#define VEND1_GLOBAL_CFG_SERDES_MODE		GENMASK(2, 0)
+#define VEND1_GLOBAL_CFG_SERDES_MODE_XFI	0
+#define VEND1_GLOBAL_CFG_SERDES_MODE_SGMII	3
+#define VEND1_GLOBAL_CFG_SERDES_MODE_OCSGMII	4
+#define VEND1_GLOBAL_CFG_SERDES_MODE_XFI5G	6
 #define VEND1_GLOBAL_CFG_RATE_ADAPT		GENMASK(8, 7)
 #define VEND1_GLOBAL_CFG_RATE_ADAPT_NONE	0
 #define VEND1_GLOBAL_CFG_RATE_ADAPT_USX		1

drivers/net/phy/aquantia/aquantia_main.c

···
 	return aqr107_wait_processor_intensive_op(phydev);
 }
 
+static const u16 aqr_global_cfg_regs[] = {
+	VEND1_GLOBAL_CFG_10M,
+	VEND1_GLOBAL_CFG_100M,
+	VEND1_GLOBAL_CFG_1G,
+	VEND1_GLOBAL_CFG_2_5G,
+	VEND1_GLOBAL_CFG_5G,
+	VEND1_GLOBAL_CFG_10G
+};
+
+static int aqr107_fill_interface_modes(struct phy_device *phydev)
+{
+	unsigned long *possible = phydev->possible_interfaces;
+	unsigned int serdes_mode, rate_adapt;
+	phy_interface_t interface;
+	int i, val;
+
+	/* Walk the media-speed configuration registers to determine which
+	 * host-side serdes modes may be used by the PHY depending on the
+	 * negotiated media speed.
+	 */
+	for (i = 0; i < ARRAY_SIZE(aqr_global_cfg_regs); i++) {
+		val = phy_read_mmd(phydev, MDIO_MMD_VEND1,
+				   aqr_global_cfg_regs[i]);
+		if (val < 0)
+			return val;
+
+		serdes_mode = FIELD_GET(VEND1_GLOBAL_CFG_SERDES_MODE, val);
+		rate_adapt = FIELD_GET(VEND1_GLOBAL_CFG_RATE_ADAPT, val);
+
+		switch (serdes_mode) {
+		case VEND1_GLOBAL_CFG_SERDES_MODE_XFI:
+			if (rate_adapt == VEND1_GLOBAL_CFG_RATE_ADAPT_USX)
+				interface = PHY_INTERFACE_MODE_USXGMII;
+			else
+				interface = PHY_INTERFACE_MODE_10GBASER;
+			break;
+
+		case VEND1_GLOBAL_CFG_SERDES_MODE_XFI5G:
+			interface = PHY_INTERFACE_MODE_5GBASER;
+			break;
+
+		case VEND1_GLOBAL_CFG_SERDES_MODE_OCSGMII:
+			interface = PHY_INTERFACE_MODE_2500BASEX;
+			break;
+
+		case VEND1_GLOBAL_CFG_SERDES_MODE_SGMII:
+			interface = PHY_INTERFACE_MODE_SGMII;
+			break;
+
+		default:
+			phydev_warn(phydev, "unrecognised serdes mode %u\n",
+				    serdes_mode);
+			interface = PHY_INTERFACE_MODE_NA;
+			break;
+		}
+
+		if (interface != PHY_INTERFACE_MODE_NA)
+			__set_bit(interface, possible);
+	}
+
+	return 0;
+}
+
+static int aqr113c_config_init(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = aqr107_config_init(phydev);
+	if (ret < 0)
+		return ret;
+
+	return aqr107_fill_interface_modes(phydev);
+}
+
 static int aqr107_probe(struct phy_device *phydev)
 {
 	int ret;
···
 	.name           = "Aquantia AQR113C",
 	.probe          = aqr107_probe,
 	.get_rate_matching = aqr107_get_rate_matching,
-	.config_init    = aqr107_config_init,
+	.config_init    = aqr113c_config_init,
 	.config_aneg    = aqr_config_aneg,
 	.config_intr    = aqr_config_intr,
 	.handle_interrupt       = aqr_handle_interrupt,

drivers/net/phy/bcm84881.c

···
 					 100000, 2000000, false);
 }
 
+static void bcm84881_fill_possible_interfaces(struct phy_device *phydev)
+{
+	unsigned long *possible = phydev->possible_interfaces;
+
+	__set_bit(PHY_INTERFACE_MODE_SGMII, possible);
+	__set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
+	__set_bit(PHY_INTERFACE_MODE_10GBASER, possible);
+}
+
 static int bcm84881_config_init(struct phy_device *phydev)
 {
+	bcm84881_fill_possible_interfaces(phydev);
+
 	switch (phydev->interface) {
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_2500BASEX:
···
 	default:
 		return -ENODEV;
 	}
+
 	return 0;
 }
 

drivers/net/phy/marvell10g.c

···
 	MV_V2_TEMP_UNKNOWN	= 0x9600, /* unknown function */
 };
 
+struct mv3310_mactype {
+	bool valid;
+	bool fixed_interface;
+	phy_interface_t interface_10g;
+};
+
 struct mv3310_chip {
 	bool (*has_downshift)(struct phy_device *phydev);
 	void (*init_supported_interfaces)(unsigned long *mask);
 	int (*get_mactype)(struct phy_device *phydev);
 	int (*set_mactype)(struct phy_device *phydev, int mactype);
 	int (*select_mactype)(unsigned long *interfaces);
-	int (*init_interface)(struct phy_device *phydev, int mactype);
+
+	const struct mv3310_mactype *mactypes;
+	size_t n_mactypes;
 
 #ifdef CONFIG_HWMON
 	int (*hwmon_read_temp_reg)(struct phy_device *phydev);
···
 
 struct mv3310_priv {
 	DECLARE_BITMAP(supported_interfaces, PHY_INTERFACE_MODE_MAX);
+	const struct mv3310_mactype *mactype;
 
 	u32 firmware_ver;
 	bool has_downshift;
-	bool rate_match;
-	phy_interface_t const_interface;
 
 	struct device *hwmon_dev;
 	char *hwmon_name;
···
 		return -1;
 }
 
-static int mv2110_init_interface(struct phy_device *phydev, int mactype)
+static const struct mv3310_mactype mv2110_mactypes[] = {
+	[MV_PMA_21X0_PORT_CTRL_MACTYPE_USXGMII] = {
+		.valid = true,
+		.fixed_interface = true,
+		.interface_10g = PHY_INTERFACE_MODE_USXGMII,
+	},
+	[MV_PMA_21X0_PORT_CTRL_MACTYPE_5GBASER] = {
+		.valid = true,
+		.interface_10g = PHY_INTERFACE_MODE_NA,
+	},
+	[MV_PMA_21X0_PORT_CTRL_MACTYPE_5GBASER_NO_SGMII_AN] = {
+		.valid = true,
+		.interface_10g = PHY_INTERFACE_MODE_NA,
+	},
+	[MV_PMA_21X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH] = {
+		.valid = true,
+		.fixed_interface = true,
+		.interface_10g = PHY_INTERFACE_MODE_10GBASER,
+	},
+};
+
+static const struct mv3310_mactype mv3310_mactypes[] = {
+	[MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI] = {
+		.valid = true,
+		.interface_10g = PHY_INTERFACE_MODE_RXAUI,
+	},
+	[MV_V2_3310_PORT_CTRL_MACTYPE_XAUI_RATE_MATCH] = {
+		.valid = true,
+		.fixed_interface = true,
+		.interface_10g = PHY_INTERFACE_MODE_XAUI,
+	},
+	[MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI_RATE_MATCH] = {
+		.valid = true,
+		.fixed_interface = true,
+		.interface_10g = PHY_INTERFACE_MODE_RXAUI,
+	},
+	[MV_V2_3310_PORT_CTRL_MACTYPE_XAUI] = {
+		.valid = true,
+		.interface_10g = PHY_INTERFACE_MODE_XAUI,
+	},
+	[MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER] = {
+		.valid = true,
+		.interface_10g = PHY_INTERFACE_MODE_10GBASER,
+	},
+	[MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_NO_SGMII_AN] = {
+		.valid = true,
+		.interface_10g = PHY_INTERFACE_MODE_10GBASER,
+	},
+	[MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH] = {
+		.valid = true,
+		.fixed_interface = true,
+		.interface_10g = PHY_INTERFACE_MODE_10GBASER,
+	},
+	[MV_V2_33X0_PORT_CTRL_MACTYPE_USXGMII] = {
+		.valid = true,
+		.fixed_interface = true,
+		.interface_10g = PHY_INTERFACE_MODE_USXGMII,
+	},
+};
+
+static const struct mv3310_mactype mv3340_mactypes[] = {
+	[MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI] = {
+		.valid = true,
+		.interface_10g = PHY_INTERFACE_MODE_RXAUI,
+	},
+	[MV_V2_3340_PORT_CTRL_MACTYPE_RXAUI_NO_SGMII_AN] = {
+		.valid = true,
+		.interface_10g = PHY_INTERFACE_MODE_RXAUI,
+	},
+	[MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI_RATE_MATCH] = {
+		.valid = true,
+		.fixed_interface = true,
+		.interface_10g = PHY_INTERFACE_MODE_RXAUI,
+	},
+	[MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER] = {
+		.valid = true,
+		.interface_10g = PHY_INTERFACE_MODE_10GBASER,
+	},
+	[MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_NO_SGMII_AN] = {
+		.valid = true,
+		.interface_10g = PHY_INTERFACE_MODE_10GBASER,
+	},
+	[MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH] = {
+		.valid = true,
+		.fixed_interface = true,
+		.interface_10g = PHY_INTERFACE_MODE_10GBASER,
+	},
+	[MV_V2_33X0_PORT_CTRL_MACTYPE_USXGMII] = {
+		.valid = true,
+		.fixed_interface = true,
+		.interface_10g = PHY_INTERFACE_MODE_USXGMII,
+	},
+};
+
+static void mv3310_fill_possible_interfaces(struct phy_device *phydev)
 {
 	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
+	unsigned long *possible = phydev->possible_interfaces;
+	const struct mv3310_mactype *mactype = priv->mactype;
 
-	priv->rate_match = false;
+	if (mactype->interface_10g != PHY_INTERFACE_MODE_NA)
+		__set_bit(priv->mactype->interface_10g, possible);
 
-	if (mactype == MV_PMA_21X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH)
-		priv->rate_match = true;
-
-	if (mactype == MV_PMA_21X0_PORT_CTRL_MACTYPE_USXGMII)
-		priv->const_interface = PHY_INTERFACE_MODE_USXGMII;
-	else if (mactype == MV_PMA_21X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH)
-		priv->const_interface = PHY_INTERFACE_MODE_10GBASER;
-	else if (mactype == MV_PMA_21X0_PORT_CTRL_MACTYPE_5GBASER ||
-		 mactype == MV_PMA_21X0_PORT_CTRL_MACTYPE_5GBASER_NO_SGMII_AN)
-		priv->const_interface = PHY_INTERFACE_MODE_NA;
-	else
-		return -EINVAL;
-
-	return 0;
-}
-
-static int mv3310_init_interface(struct phy_device *phydev, int mactype)
-{
-	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
-
-	priv->rate_match = false;
-
-	if (mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH ||
-	    mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI_RATE_MATCH ||
-	    mactype == MV_V2_3310_PORT_CTRL_MACTYPE_XAUI_RATE_MATCH)
-		priv->rate_match = true;
-
-	if (mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_USXGMII)
-		priv->const_interface = PHY_INTERFACE_MODE_USXGMII;
-	else if (mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_RATE_MATCH ||
-		 mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER_NO_SGMII_AN ||
-		 mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_10GBASER)
-		priv->const_interface = PHY_INTERFACE_MODE_10GBASER;
-	else if (mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI_RATE_MATCH ||
-		 mactype == MV_V2_33X0_PORT_CTRL_MACTYPE_RXAUI)
-		priv->const_interface = PHY_INTERFACE_MODE_RXAUI;
-	else if (mactype == MV_V2_3310_PORT_CTRL_MACTYPE_XAUI_RATE_MATCH ||
-		 mactype == MV_V2_3310_PORT_CTRL_MACTYPE_XAUI)
-		priv->const_interface = PHY_INTERFACE_MODE_XAUI;
-	else
-		return -EINVAL;
-
-	return 0;
-}
-
-static int mv3340_init_interface(struct phy_device *phydev, int mactype)
-{
-	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
-	int err = 0;
-
-	priv->rate_match = false;
-
-	if (mactype == MV_V2_3340_PORT_CTRL_MACTYPE_RXAUI_NO_SGMII_AN)
-		priv->const_interface = PHY_INTERFACE_MODE_RXAUI;
-	else
-		err = mv3310_init_interface(phydev, mactype);
-
-	return err;
+	if (!mactype->fixed_interface) {
+		__set_bit(PHY_INTERFACE_MODE_5GBASER, possible);
+		__set_bit(PHY_INTERFACE_MODE_2500BASEX, possible);
+		__set_bit(PHY_INTERFACE_MODE_SGMII, possible);
+	}
 }
 
 static int mv3310_config_init(struct phy_device *phydev)
···
 	if (mactype < 0)
 		return mactype;
 
-	err = chip->init_interface(phydev, mactype);
-	if (err) {
+	if (mactype >= chip->n_mactypes || !chip->mactypes[mactype].valid) {
 		phydev_err(phydev, "MACTYPE configuration invalid\n");
-		return err;
+		return -EINVAL;
 	}
+
+	priv->mactype = &chip->mactypes[mactype];
+
+	mv3310_fill_possible_interfaces(phydev);
 
 	/* Enable EDPD mode - saving 600mW */
 	err = mv3310_set_edpd(phydev, ETHTOOL_PHY_EDPD_DFLT_TX_MSECS);
···
 	 *
 	 * In USXGMII mode the PHY interface mode is also fixed.
 	 */
-	if (priv->rate_match ||
-	    priv->const_interface == PHY_INTERFACE_MODE_USXGMII) {
-		phydev->interface = priv->const_interface;
+	if (priv->mactype->fixed_interface) {
+		phydev->interface = priv->mactype->interface_10g;
 		return;
 	}
 
···
 	 */
 	switch (phydev->speed) {
 	case SPEED_10000:
-		phydev->interface = priv->const_interface;
+		phydev->interface = priv->mactype->interface_10g;
 		break;
 	case SPEED_5000:
 		phydev->interface = PHY_INTERFACE_MODE_5GBASER;
···
 	.get_mactype = mv3310_get_mactype,
 	.set_mactype = mv3310_set_mactype,
 	.select_mactype = mv3310_select_mactype,
-	.init_interface = mv3310_init_interface,
+
+	.mactypes = mv3310_mactypes,
+	.n_mactypes = ARRAY_SIZE(mv3310_mactypes),
 
 #ifdef CONFIG_HWMON
 	.hwmon_read_temp_reg = mv3310_hwmon_read_temp_reg,
···
 	.get_mactype = mv3310_get_mactype,
 	.set_mactype = mv3310_set_mactype,
 	.select_mactype = mv3310_select_mactype,
-	.init_interface = mv3340_init_interface,
+
+	.mactypes = mv3340_mactypes,
+	.n_mactypes = ARRAY_SIZE(mv3340_mactypes),
 
 #ifdef CONFIG_HWMON
 	.hwmon_read_temp_reg = mv3310_hwmon_read_temp_reg,
···
 	.get_mactype = mv2110_get_mactype,
 	.set_mactype = mv2110_set_mactype,
 	.select_mactype = mv2110_select_mactype,
-	.init_interface = mv2110_init_interface,
+
+	.mactypes = mv2110_mactypes,
+	.n_mactypes = ARRAY_SIZE(mv2110_mactypes),
 
 #ifdef CONFIG_HWMON
 	.hwmon_read_temp_reg = mv2110_hwmon_read_temp_reg,
···
 	.get_mactype = mv2110_get_mactype,
 	.set_mactype = mv2110_set_mactype,
 	.select_mactype = mv2110_select_mactype,
-	.init_interface = mv2110_init_interface,
+
+	.mactypes = mv2110_mactypes,
+	.n_mactypes = ARRAY_SIZE(mv2110_mactypes),
 
 #ifdef CONFIG_HWMON
 	.hwmon_read_temp_reg = mv2110_hwmon_read_temp_reg,

drivers/net/phy/phy_device.c

···
 	if (ret < 0)
 		return ret;
 
+	phy_interface_zero(phydev->possible_interfaces);
+
 	if (phydev->drv->config_init) {
 		ret = phydev->drv->config_init(phydev);
 		if (ret < 0)

drivers/net/phy/phylink.c

···
 })
 #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
···
 	return phylink_is_empty_linkmode(supported) ? -EINVAL : 0;
 }
 
-static int phylink_validate_mask(struct phylink *pl, unsigned long *supported,
+static void phylink_validate_one(struct phylink *pl, struct phy_device *phy,
+				 const unsigned long *supported,
+				 const struct phylink_link_state *state,
+				 phy_interface_t interface,
+				 unsigned long *accum_supported,
+				 unsigned long *accum_advertising)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(tmp_supported);
+	struct phylink_link_state tmp_state;
+
+	linkmode_copy(tmp_supported, supported);
+
+	tmp_state = *state;
+	tmp_state.interface = interface;
+
+	if (phy)
+		tmp_state.rate_matching = phy_get_rate_matching(phy, interface);
+
+	if (!phylink_validate_mac_and_pcs(pl, tmp_supported, &tmp_state)) {
+		phylink_dbg(pl, " interface %u (%s) rate match %s supports %*pbl\n",
+			    interface, phy_modes(interface),
+			    phy_rate_matching_to_str(tmp_state.rate_matching),
+			    __ETHTOOL_LINK_MODE_MASK_NBITS, tmp_supported);
+
+		linkmode_or(accum_supported, accum_supported, tmp_supported);
+		linkmode_or(accum_advertising, accum_advertising,
+			    tmp_state.advertising);
+	}
+}
+
+static int phylink_validate_mask(struct phylink *pl, struct phy_device *phy,
+				 unsigned long *supported,
 				 struct phylink_link_state *state,
 				 const unsigned long *interfaces)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(all_adv) = { 0, };
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(all_s) = { 0, };
-	__ETHTOOL_DECLARE_LINK_MODE_MASK(s);
-	struct phylink_link_state t;
 	int interface;
 
-	for_each_set_bit(interface, interfaces, PHY_INTERFACE_MODE_MAX) {
-		linkmode_copy(s, supported);
-
-		t = *state;
-		t.interface = interface;
-		if (!phylink_validate_mac_and_pcs(pl, s, &t)) {
-			linkmode_or(all_s, all_s, s);
-			linkmode_or(all_adv, all_adv, t.advertising);
-		}
-	}
+	for_each_set_bit(interface, interfaces, PHY_INTERFACE_MODE_MAX)
+		phylink_validate_one(pl, phy, supported, state, interface,
+				     all_s, all_adv);
 
 	linkmode_copy(supported, all_s);
 	linkmode_copy(state->advertising, all_adv);
···
 	const unsigned long *interfaces = pl->config->supported_interfaces;
 
 	if (state->interface == PHY_INTERFACE_MODE_NA)
-		return phylink_validate_mask(pl, supported, state, interfaces);
+		return phylink_validate_mask(pl, NULL, supported, state,
+					     interfaces);
 
 	if (!test_bit(state->interface, interfaces))
 		return -EINVAL;
···
 		    phylink_pause_to_str(pl->phy_state.pause));
 }
 
+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.
+	 */
+	state->rate_matching = phy_get_rate_matching(phy, state->interface);
+
+	/* Clause 45 PHYs may switch their Serdes lane between, e.g. 10GBASE-R,
+	 * 5GBASE-R, 2500BASE-X and SGMII if they are not using rate matching.
+	 * For some interface modes (e.g. RXAUI, XAUI and USXGMII) switching
+	 * their Serdes is either unnecessary or not reasonable.
+	 *
+	 * For these which switch interface modes, we really need to know which
+	 * interface modes the PHY supports to properly work out which ethtool
+	 * linkmodes can be supported. For now, as a work-around, we validate
+	 * against all interface modes, which may lead to more ethtool link
+	 * modes being advertised than are actually supported.
+	 */
+	if (phy->is_c45 && state->rate_matching == RATE_MATCH_NONE &&
+	    state->interface != PHY_INTERFACE_MODE_RXAUI &&
+	    state->interface != PHY_INTERFACE_MODE_XAUI &&
+	    state->interface != PHY_INTERFACE_MODE_USXGMII)
+		state->interface = PHY_INTERFACE_MODE_NA;
+
+	return phylink_validate(pl, supported, state);
+}
+
 static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy,
 			       phy_interface_t interface)
 {
···
 	memset(&config, 0, sizeof(config));
 	linkmode_copy(supported, phy->supported);
 	linkmode_copy(config.advertising, phy->advertising);
+	config.interface = interface;
 
-	/* Check whether we would use rate matching for the proposed interface
-	 * mode.
-	 */
-	config.rate_matching = phy_get_rate_matching(phy, interface);
-
-	/* Clause 45 PHYs may switch their Serdes lane between, e.g. 10GBASE-R,
-	 * 5GBASE-R, 2500BASE-X and SGMII if they are not using rate matching.
-	 * For some interface modes (e.g. RXAUI, XAUI and USXGMII) switching
-	 * their Serdes is either unnecessary or not reasonable.
-	 *
-	 * For these which switch interface modes, we really need to know which
-	 * interface modes the PHY supports to properly work out which ethtool
-	 * linkmodes can be supported. For now, as a work-around, we validate
-	 * against all interface modes, which may lead to more ethtool link
-	 * modes being advertised than are actually supported.
-	 */
-	if (phy->is_c45 && config.rate_matching == RATE_MATCH_NONE &&
-	    interface != PHY_INTERFACE_MODE_RXAUI &&
-	    interface != PHY_INTERFACE_MODE_XAUI &&
-	    interface != PHY_INTERFACE_MODE_USXGMII)
-		config.interface = PHY_INTERFACE_MODE_NA;
-	else
-		config.interface = interface;
-
-	ret = phylink_validate(pl, supported, &config);
+	ret = phylink_validate_phy(pl, phy, supported, &config);
 	if (ret) {
 		phylink_warn(pl, "validation of %s with support %*pb and advertisement %*pb failed: %pe\n",
 			     phy_modes(config.interface),
···
 	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)
 {
···
 	/* For all the interfaces that are supported, reduce the sfp_support
 	 * mask to only those link modes that can be supported.
 	 */
-	ret = phylink_validate_mask(pl, pl->sfp_support, &config, interfaces);
+	ret = phylink_validate_mask(pl, NULL, pl->sfp_support, &config,
+				    interfaces);
 	if (ret) {
 		phylink_err(pl, "unsupported SFP module: validation with support %*pb failed\n",
 			    __ETHTOOL_LINK_MODE_MASK_NBITS, support);

include/linux/phy.h

···
  * @irq_rerun: Flag indicating interrupts occurred while PHY was suspended,
  *             requiring a rerun of the interrupt handler after resume
  * @interface: enum phy_interface_t value
+ * @possible_interfaces: bitmap if interface modes that the attached PHY
+ *			 will switch between depending on media speed.
  * @skb: Netlink message for cable diagnostics
  * @nest: Netlink nest used for cable diagnostics
  * @ehdr: nNtlink header for cable diagnostics
···
 	u32 dev_flags;
 
 	phy_interface_t interface;
+	DECLARE_PHY_INTERFACE_MASK(possible_interfaces);
 
 	/*
 	 * forced speed & duplex (no autoneg)