[RFC PATCH net-next 0/8] Convert xpcs to phylink_pcs_ops

[RFC PATCH net-next 0/8] Convert xpcs to phylink_pcs_ops

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

This patch series is COMPLETELY UNTESTED (I don't have stmmac hardware
with the xpcs) hence the RFC tag. If people from Intel could test this
it would be great.

Background: the sja1105 DSA driver currently drives a Designware XPCS
for SGMII and 2500base-X, and it would be nice to reuse some code with
the xpcs module. This would also help consolidate the phylink_pcs_ops,
since the only user of that, currently, is the lynx_pcs.

Therefore, this series makes the xpcs expose the same kind of API that
the lynx_pcs module does.

Note: this patch series must be applied on top of:
https://patchwork.kernel.org/project/netdevbpf/patch/20210527155959.3270478-1-olteanv@gmail.com/

Vladimir Oltean (8):
  net: pcs: xpcs: delete shim definition for mdio_xpcs_get_ops()
  net: pcs: xpcs: check for supported PHY interface modes in
    phylink_validate
  net: pcs: xpcs: export xpcs_validate
  net: pcs: export xpcs_config_eee
  net: pcs: xpcs: export xpcs_probe
  net: pcs: xpcs: convert to phylink_pcs_ops
  net: pcs: xpcs: use mdiobus_c45_addr in xpcs_{read,write}
  net: pcs: xpcs: convert to mdio_device

 drivers/net/ethernet/stmicro/stmmac/common.h  |   3 +-
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  14 --
 .../ethernet/stmicro/stmmac/stmmac_ethtool.c  |   5 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  41 +---
 .../net/ethernet/stmicro/stmmac/stmmac_mdio.c |  41 ++--
 drivers/net/pcs/pcs-xpcs.c                    | 199 +++++++++++-------
 include/linux/pcs/pcs-xpcs.h                  |  35 +--
 7 files changed, 162 insertions(+), 176 deletions(-)

-- 
2.25.1

[RFC PATCH net-next 1/8] net: pcs: xpcs: delete shim definition for mdio_xpcs_get_ops()

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

CONFIG_STMMAC_ETH selects CONFIG_PCS_XPCS, so there should be no
situation where the shim should be needed.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 include/linux/pcs/pcs-xpcs.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index 5938ced805f4..c4d0a2c469c7 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -36,13 +36,6 @@ struct mdio_xpcs_ops {
 			  int enable);
 };
 
-#if IS_ENABLED(CONFIG_PCS_XPCS)
 struct mdio_xpcs_ops *mdio_xpcs_get_ops(void);
-#else
-static inline struct mdio_xpcs_ops *mdio_xpcs_get_ops(void)
-{
-	return NULL;
-}
-#endif
 
 #endif /* __LINUX_PCS_XPCS_H */
-- 
2.25.1

[RFC PATCH net-next 2/8] net: pcs: xpcs: check for supported PHY interface modes in phylink_validate

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The supported PHY interface types are currently deduced by reading the
PHY ID registers of the PCS, to determine whether it is an USXGMII,
10G-KR, XLGMII, SGMII PCS or whatever.

Checking whether the PCS operates in a PHY interface mode compatible
with the hardware capability is done only once: in xpcs_check_features,
called from xpcs_probe - the deduced PHY interface mode is compared to
what is specified in the device tree.

But nothing prevents phylink from changing the state->interface as a
result of plugging an SFP module with a different operating PHY
interface type.

This change deletes xpcs_check_features, removes the phy_interface_t
argument from xpcs_probe, and moves the PHY interface type check inside
the validate function, similar to what other drivers do.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_mdio.c |  5 +-
 drivers/net/pcs/pcs-xpcs.c                    | 63 ++++++++++---------
 include/linux/pcs/pcs-xpcs.h                  |  5 +-
 3 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index e293bf1ce9f3..d12dfa60b8ea 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -512,7 +512,8 @@ int stmmac_mdio_register(struct net_device *ndev)
 	/* Try to probe the XPCS by scanning all addresses. */
 	if (priv->hw->xpcs) {
 		struct mdio_xpcs_args *xpcs = &priv->hw->xpcs_args;
-		int ret, mode = priv->plat->phy_interface;
+		int ret;
+
 		max_addr = PHY_MAX_ADDR;
 
 		xpcs->bus = new_bus;
@@ -521,7 +522,7 @@ int stmmac_mdio_register(struct net_device *ndev)
 		for (addr = 0; addr < max_addr; addr++) {
 			xpcs->addr = addr;
 
-			ret = stmmac_xpcs_probe(priv, xpcs, mode);
+			ret = stmmac_xpcs_probe(priv, xpcs);
 			if (!ret) {
 				found = 1;
 				break;
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index aa985a5aae8d..71efd5ef69e5 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -662,6 +662,30 @@ static int xpcs_validate(struct mdio_xpcs_args *xpcs,
 			 unsigned long *supported,
 			 struct phylink_link_state *state)
 {
+	bool valid_interface;
+
+	if (state->interface == PHY_INTERFACE_MODE_NA) {
+		valid_interface = true;
+	} else {
+		struct xpcs_id *id = xpcs->id;
+		int i;
+
+		valid_interface = false;
+
+		for (i = 0; id->interface[i] != PHY_INTERFACE_MODE_MAX; i++) {
+			if (id->interface[i] != state->interface)
+				continue;
+
+			valid_interface = true;
+			break;
+		}
+	}
+
+	if (!valid_interface) {
+		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+		return 0;
+	}
+
 	linkmode_and(supported, supported, xpcs->supported);
 	linkmode_and(state->advertising, state->advertising, xpcs->supported);
 	return 0;
@@ -910,43 +934,24 @@ static u32 xpcs_get_id(struct mdio_xpcs_args *xpcs)
 	return 0xffffffff;
 }
 
-static bool xpcs_check_features(struct mdio_xpcs_args *xpcs,
-				struct xpcs_id *match,
-				phy_interface_t interface)
-{
-	int i;
-
-	for (i = 0; match->interface[i] != PHY_INTERFACE_MODE_MAX; i++) {
-		if (match->interface[i] == interface)
-			break;
-	}
-
-	if (match->interface[i] == PHY_INTERFACE_MODE_MAX)
-		return false;
-
-	for (i = 0; match->supported[i] != __ETHTOOL_LINK_MODE_MASK_NBITS; i++)
-		set_bit(match->supported[i], xpcs->supported);
-
-	xpcs->an_mode = match->an_mode;
-
-	return true;
-}
-
-static int xpcs_probe(struct mdio_xpcs_args *xpcs, phy_interface_t interface)
+static int xpcs_probe(struct mdio_xpcs_args *xpcs)
 {
 	u32 xpcs_id = xpcs_get_id(xpcs);
-	struct xpcs_id *match = NULL;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(xpcs_id_list); i++) {
 		struct xpcs_id *entry = &xpcs_id_list[i];
 
-		if ((xpcs_id & entry->mask) == entry->id) {
-			match = entry;
+		if ((xpcs_id & entry->mask) != entry->id)
+			continue;
 
-			if (xpcs_check_features(xpcs, match, interface))
-				return xpcs_soft_reset(xpcs);
-		}
+		for (i = 0; entry->supported[i] != __ETHTOOL_LINK_MODE_MASK_NBITS; i++)
+			set_bit(entry->supported[i], xpcs->supported);
+
+		xpcs->id = entry;
+		xpcs->an_mode = entry->an_mode;
+
+		return xpcs_soft_reset(xpcs);
 	}
 
 	return -ENODEV;
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index c4d0a2c469c7..e48636a1a078 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -14,9 +14,12 @@
 #define DW_AN_C73			1
 #define DW_AN_C37_SGMII			2
 
+struct xpcs_id;
+
 struct mdio_xpcs_args {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
 	struct mii_bus *bus;
+	struct xpcs_id *id;
 	int addr;
 	int an_mode;
 };
@@ -31,7 +34,7 @@ struct mdio_xpcs_ops {
 			 struct phylink_link_state *state);
 	int (*link_up)(struct mdio_xpcs_args *xpcs, int speed,
 		       phy_interface_t interface);
-	int (*probe)(struct mdio_xpcs_args *xpcs, phy_interface_t interface);
+	int (*probe)(struct mdio_xpcs_args *xpcs);
 	int (*config_eee)(struct mdio_xpcs_args *xpcs, int mult_fact_100ns,
 			  int enable);
 };
-- 
2.25.1

[RFC PATCH net-next 3/8] net: pcs: xpcs: export xpcs_validate

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Calling a function pointer with a single implementation through
struct mdio_xpcs_ops is clunky, and the stmmac_do_callback system forces
this to return int, even though it always returns zero.

Simply remove the "validate" function pointer from struct mdio_xpcs_ops
and replace it with an exported xpcs_validate symbol which is called
directly by stmmac.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/hwif.h        |  2 --
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  2 +-
 drivers/net/pcs/pcs-xpcs.c                        | 10 ++++------
 include/linux/pcs/pcs-xpcs.h                      |  5 ++---
 4 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 75a8b90c202a..a86b358feae9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -613,8 +613,6 @@ struct stmmac_mmc_ops {
 	stmmac_do_void_callback(__priv, mmc, read, __args)
 
 /* XPCS callbacks */
-#define stmmac_xpcs_validate(__priv, __args...) \
-	stmmac_do_callback(__priv, xpcs, validate, __args)
 #define stmmac_xpcs_config(__priv, __args...) \
 	stmmac_do_callback(__priv, xpcs, config, __args)
 #define stmmac_xpcs_get_state(__priv, __args...) \
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index bf9fe25fed69..d3d85d36e177 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -996,7 +996,7 @@ static void stmmac_validate(struct phylink_config *config,
 	linkmode_andnot(state->advertising, state->advertising, mask);
 
 	/* If PCS is supported, check which modes it supports. */
-	stmmac_xpcs_validate(priv, &priv->hw->xpcs_args, supported, state);
+	xpcs_validate(&priv->hw->xpcs_args, supported, state);
 }
 
 static void stmmac_mac_pcs_get_state(struct phylink_config *config,
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 71efd5ef69e5..4c0093473470 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -658,9 +658,8 @@ static void xpcs_resolve_pma(struct mdio_xpcs_args *xpcs,
 	}
 }
 
-static int xpcs_validate(struct mdio_xpcs_args *xpcs,
-			 unsigned long *supported,
-			 struct phylink_link_state *state)
+void xpcs_validate(struct mdio_xpcs_args *xpcs, unsigned long *supported,
+		   struct phylink_link_state *state)
 {
 	bool valid_interface;
 
@@ -683,13 +682,13 @@ static int xpcs_validate(struct mdio_xpcs_args *xpcs,
 
 	if (!valid_interface) {
 		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
-		return 0;
+		return;
 	}
 
 	linkmode_and(supported, supported, xpcs->supported);
 	linkmode_and(state->advertising, state->advertising, xpcs->supported);
-	return 0;
 }
+EXPORT_SYMBOL_GPL(xpcs_validate);
 
 static int xpcs_config_eee(struct mdio_xpcs_args *xpcs, int mult_fact_100ns,
 			   int enable)
@@ -958,7 +957,6 @@ static int xpcs_probe(struct mdio_xpcs_args *xpcs)
 }
 
 static struct mdio_xpcs_ops xpcs_ops = {
-	.validate = xpcs_validate,
 	.config = xpcs_config,
 	.get_state = xpcs_get_state,
 	.link_up = xpcs_link_up,
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index e48636a1a078..56755b7895a0 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -25,9 +25,6 @@ struct mdio_xpcs_args {
 };
 
 struct mdio_xpcs_ops {
-	int (*validate)(struct mdio_xpcs_args *xpcs,
-			unsigned long *supported,
-			struct phylink_link_state *state);
 	int (*config)(struct mdio_xpcs_args *xpcs,
 		      const struct phylink_link_state *state);
 	int (*get_state)(struct mdio_xpcs_args *xpcs,
@@ -40,5 +37,7 @@ struct mdio_xpcs_ops {
 };
 
 struct mdio_xpcs_ops *mdio_xpcs_get_ops(void);
+void xpcs_validate(struct mdio_xpcs_args *xpcs, unsigned long *supported,
+		   struct phylink_link_state *state);
 
 #endif /* __LINUX_PCS_XPCS_H */
-- 
2.25.1

[RFC PATCH net-next 4/8] net: pcs: export xpcs_config_eee

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

There is no good reason why we need to go through:

stmmac_xpcs_config_eee
-> stmmac_do_callback
   -> mdio_xpcs_ops->config_eee
      -> xpcs_config_eee

when we can simply call xpcs_config_eee.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/hwif.h           | 2 --
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 6 +++---
 drivers/net/pcs/pcs-xpcs.c                           | 6 +++---
 include/linux/pcs/pcs-xpcs.h                         | 4 ++--
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index a86b358feae9..2d2843edaf21 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -621,8 +621,6 @@ struct stmmac_mmc_ops {
 	stmmac_do_callback(__priv, xpcs, link_up, __args)
 #define stmmac_xpcs_probe(__priv, __args...) \
 	stmmac_do_callback(__priv, xpcs, probe, __args)
-#define stmmac_xpcs_config_eee(__priv, __args...) \
-	stmmac_do_callback(__priv, xpcs, config_eee, __args)
 
 struct stmmac_regs_off {
 	u32 ptp_off;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 1f6d749fd9a3..72d2d575bbfe 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -720,9 +720,9 @@ static int stmmac_ethtool_op_set_eee(struct net_device *dev,
 		netdev_warn(priv->dev,
 			    "Setting EEE tx-lpi is not supported\n");
 
-	ret = stmmac_xpcs_config_eee(priv, &priv->hw->xpcs_args,
-				     priv->plat->mult_fact_100ns,
-				     edata->eee_enabled);
+	ret = xpcs_config_eee(&priv->hw->xpcs_args,
+			      priv->plat->mult_fact_100ns,
+			      edata->eee_enabled);
 	if (ret)
 		return ret;
 
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 4c0093473470..a7851a8a219b 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -690,8 +690,8 @@ void xpcs_validate(struct mdio_xpcs_args *xpcs, unsigned long *supported,
 }
 EXPORT_SYMBOL_GPL(xpcs_validate);
 
-static int xpcs_config_eee(struct mdio_xpcs_args *xpcs, int mult_fact_100ns,
-			   int enable)
+int xpcs_config_eee(struct mdio_xpcs_args *xpcs, int mult_fact_100ns,
+		    int enable)
 {
 	int ret;
 
@@ -722,6 +722,7 @@ static int xpcs_config_eee(struct mdio_xpcs_args *xpcs, int mult_fact_100ns,
 	ret |= DW_VR_MII_EEE_TRN_LPI;
 	return xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_EEE_MCTRL1, ret);
 }
+EXPORT_SYMBOL_GPL(xpcs_config_eee);
 
 static int xpcs_config_aneg_c37_sgmii(struct mdio_xpcs_args *xpcs)
 {
@@ -961,7 +962,6 @@ static struct mdio_xpcs_ops xpcs_ops = {
 	.get_state = xpcs_get_state,
 	.link_up = xpcs_link_up,
 	.probe = xpcs_probe,
-	.config_eee = xpcs_config_eee,
 };
 
 struct mdio_xpcs_ops *mdio_xpcs_get_ops(void)
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index 56755b7895a0..203aafae9166 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -32,12 +32,12 @@ struct mdio_xpcs_ops {
 	int (*link_up)(struct mdio_xpcs_args *xpcs, int speed,
 		       phy_interface_t interface);
 	int (*probe)(struct mdio_xpcs_args *xpcs);
-	int (*config_eee)(struct mdio_xpcs_args *xpcs, int mult_fact_100ns,
-			  int enable);
 };
 
 struct mdio_xpcs_ops *mdio_xpcs_get_ops(void);
 void xpcs_validate(struct mdio_xpcs_args *xpcs, unsigned long *supported,
 		   struct phylink_link_state *state);
+int xpcs_config_eee(struct mdio_xpcs_args *xpcs, int mult_fact_100ns,
+		    int enable);
 
 #endif /* __LINUX_PCS_XPCS_H */
-- 
2.25.1

[RFC PATCH net-next 5/8] net: pcs: xpcs: export xpcs_probe

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Similar to the other recently functions, it is not necessary for
xpcs_probe to be a function pointer, so export it so that it can be
called directly.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/hwif.h        | 2 --
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 2 +-
 drivers/net/pcs/pcs-xpcs.c                        | 4 ++--
 include/linux/pcs/pcs-xpcs.h                      | 2 +-
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 2d2843edaf21..5014b260844b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -619,8 +619,6 @@ struct stmmac_mmc_ops {
 	stmmac_do_callback(__priv, xpcs, get_state, __args)
 #define stmmac_xpcs_link_up(__priv, __args...) \
 	stmmac_do_callback(__priv, xpcs, link_up, __args)
-#define stmmac_xpcs_probe(__priv, __args...) \
-	stmmac_do_callback(__priv, xpcs, probe, __args)
 
 struct stmmac_regs_off {
 	u32 ptp_off;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index d12dfa60b8ea..2af83d902ea1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -522,7 +522,7 @@ int stmmac_mdio_register(struct net_device *ndev)
 		for (addr = 0; addr < max_addr; addr++) {
 			xpcs->addr = addr;
 
-			ret = stmmac_xpcs_probe(priv, xpcs);
+			ret = xpcs_probe(xpcs);
 			if (!ret) {
 				found = 1;
 				break;
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index a7851a8a219b..4063dcc0f767 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -934,7 +934,7 @@ static u32 xpcs_get_id(struct mdio_xpcs_args *xpcs)
 	return 0xffffffff;
 }
 
-static int xpcs_probe(struct mdio_xpcs_args *xpcs)
+int xpcs_probe(struct mdio_xpcs_args *xpcs)
 {
 	u32 xpcs_id = xpcs_get_id(xpcs);
 	int i;
@@ -956,12 +956,12 @@ static int xpcs_probe(struct mdio_xpcs_args *xpcs)
 
 	return -ENODEV;
 }
+EXPORT_SYMBOL_GPL(xpcs_probe);
 
 static struct mdio_xpcs_ops xpcs_ops = {
 	.config = xpcs_config,
 	.get_state = xpcs_get_state,
 	.link_up = xpcs_link_up,
-	.probe = xpcs_probe,
 };
 
 struct mdio_xpcs_ops *mdio_xpcs_get_ops(void)
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index 203aafae9166..11585fa093cd 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -31,7 +31,6 @@ struct mdio_xpcs_ops {
 			 struct phylink_link_state *state);
 	int (*link_up)(struct mdio_xpcs_args *xpcs, int speed,
 		       phy_interface_t interface);
-	int (*probe)(struct mdio_xpcs_args *xpcs);
 };
 
 struct mdio_xpcs_ops *mdio_xpcs_get_ops(void);
@@ -39,5 +38,6 @@ void xpcs_validate(struct mdio_xpcs_args *xpcs, unsigned long *supported,
 		   struct phylink_link_state *state);
 int xpcs_config_eee(struct mdio_xpcs_args *xpcs, int mult_fact_100ns,
 		    int enable);
+int xpcs_probe(struct mdio_xpcs_args *xpcs);
 
 #endif /* __LINUX_PCS_XPCS_H */
-- 
2.25.1

[RFC PATCH net-next 6/8] net: pcs: xpcs: convert to phylink_pcs_ops

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Since all the remaining members of struct mdio_xpcs_ops have direct
equivalents in struct phylink_pcs_ops, it is about time we remove it
altogether.

Since the phylink ops return void, we need to remove the error
propagation from the various xpcs methods and simply print an error
message where appropriate.

Since xpcs_get_state_c73() detects link faults and attempts to reset the
link on its own by calling xpcs_config(), but xpcs_config() now has a
lot of phylink arguments which are not needed and cannot be simply
fabricated by anybody else except phylink, the actual implementation has
been moved into a smaller xpcs_do_config().

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/common.h  |  1 -
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  8 --
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 37 ++------
 .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 10 +-
 drivers/net/pcs/pcs-xpcs.c                    | 94 +++++++++++--------
 include/linux/pcs/pcs-xpcs.h                  | 11 +--
 6 files changed, 67 insertions(+), 94 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 619e3c0760d6..678f8ce62b8a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -503,7 +503,6 @@ struct mac_device_info {
 	const struct stmmac_hwtimestamp *ptp;
 	const struct stmmac_tc_ops *tc;
 	const struct stmmac_mmc_ops *mmc;
-	const struct mdio_xpcs_ops *xpcs;
 	struct mdio_xpcs_args xpcs_args;
 	struct mii_regs mii;	/* MII register Addresses */
 	struct mac_link link;
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 5014b260844b..91f7592a0189 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -612,14 +612,6 @@ struct stmmac_mmc_ops {
 #define stmmac_mmc_read(__priv, __args...) \
 	stmmac_do_void_callback(__priv, mmc, read, __args)
 
-/* XPCS callbacks */
-#define stmmac_xpcs_config(__priv, __args...) \
-	stmmac_do_callback(__priv, xpcs, config, __args)
-#define stmmac_xpcs_get_state(__priv, __args...) \
-	stmmac_do_callback(__priv, xpcs, get_state, __args)
-#define stmmac_xpcs_link_up(__priv, __args...) \
-	stmmac_do_callback(__priv, xpcs, link_up, __args)
-
 struct stmmac_regs_off {
 	u32 ptp_off;
 	u32 mmc_off;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d3d85d36e177..3ccf00ea77d5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -999,28 +999,6 @@ static void stmmac_validate(struct phylink_config *config,
 	xpcs_validate(&priv->hw->xpcs_args, supported, state);
 }
 
-static void stmmac_mac_pcs_get_state(struct phylink_config *config,
-				     struct phylink_link_state *state)
-{
-	struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
-
-	state->link = 0;
-	stmmac_xpcs_get_state(priv, &priv->hw->xpcs_args, state);
-}
-
-static void stmmac_mac_config(struct phylink_config *config, unsigned int mode,
-			      const struct phylink_link_state *state)
-{
-	struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
-
-	stmmac_xpcs_config(priv, &priv->hw->xpcs_args, state);
-}
-
-static void stmmac_mac_an_restart(struct phylink_config *config)
-{
-	/* Not Supported */
-}
-
 static void stmmac_fpe_link_state_handle(struct stmmac_priv *priv, bool is_up)
 {
 	struct stmmac_fpe_cfg *fpe_cfg = priv->plat->fpe_cfg;
@@ -1060,8 +1038,6 @@ static void stmmac_mac_link_up(struct phylink_config *config,
 	struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
 	u32 ctrl;
 
-	stmmac_xpcs_link_up(priv, &priv->hw->xpcs_args, speed, interface);
-
 	ctrl = readl(priv->ioaddr + MAC_CTRL_REG);
 	ctrl &= ~priv->hw->link.speed_mask;
 
@@ -1154,9 +1130,6 @@ static void stmmac_mac_link_up(struct phylink_config *config,
 
 static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
 	.validate = stmmac_validate,
-	.mac_pcs_get_state = stmmac_mac_pcs_get_state,
-	.mac_config = stmmac_mac_config,
-	.mac_an_restart = stmmac_mac_an_restart,
 	.mac_link_down = stmmac_mac_link_down,
 	.mac_link_up = stmmac_mac_link_up,
 };
@@ -1230,6 +1203,7 @@ static int stmmac_init_phy(struct net_device *dev)
 
 static int stmmac_phy_setup(struct stmmac_priv *priv)
 {
+	struct stmmac_mdio_bus_data *mdio_bus_data = priv->plat->mdio_bus_data;
 	struct fwnode_handle *fwnode = of_fwnode_handle(priv->plat->phylink_node);
 	int mode = priv->plat->phy_interface;
 	struct phylink *phylink;
@@ -1237,8 +1211,7 @@ static int stmmac_phy_setup(struct stmmac_priv *priv)
 	priv->phylink_config.dev = &priv->dev->dev;
 	priv->phylink_config.type = PHYLINK_NETDEV;
 	priv->phylink_config.pcs_poll = true;
-	priv->phylink_config.ovr_an_inband =
-		priv->plat->mdio_bus_data->xpcs_an_inband;
+	priv->phylink_config.ovr_an_inband = mdio_bus_data->xpcs_an_inband;
 
 	if (!fwnode)
 		fwnode = dev_fwnode(priv->device);
@@ -1248,6 +1221,12 @@ static int stmmac_phy_setup(struct stmmac_priv *priv)
 	if (IS_ERR(phylink))
 		return PTR_ERR(phylink);
 
+	if (mdio_bus_data->has_xpcs) {
+		struct mdio_xpcs_args *xpcs = &priv->hw->xpcs_args;
+
+		phylink_set_pcs(phylink, &xpcs->pcs);
+	}
+
 	priv->phylink = phylink;
 	return 0;
 }
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index 2af83d902ea1..4a197b2fe26b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -444,14 +444,6 @@ int stmmac_mdio_register(struct net_device *ndev)
 		max_addr = PHY_MAX_ADDR;
 	}
 
-	if (mdio_bus_data->has_xpcs) {
-		priv->hw->xpcs = mdio_xpcs_get_ops();
-		if (!priv->hw->xpcs) {
-			err = -ENODEV;
-			goto bus_register_fail;
-		}
-	}
-
 	if (mdio_bus_data->needs_reset)
 		new_bus->reset = &stmmac_mdio_reset;
 
@@ -510,7 +502,7 @@ int stmmac_mdio_register(struct net_device *ndev)
 	}
 
 	/* Try to probe the XPCS by scanning all addresses. */
-	if (priv->hw->xpcs) {
+	if (mdio_bus_data->has_xpcs) {
 		struct mdio_xpcs_args *xpcs = &priv->hw->xpcs_args;
 		int ret;
 
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 4063dcc0f767..288abe8ddaf3 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -103,6 +103,9 @@
 /* VR MII EEE Control 1 defines */
 #define DW_VR_MII_EEE_TRN_LPI		BIT(0)	/* Transparent Mode Enable */
 
+#define phylink_pcs_to_xpcs(pl_pcs) \
+	container_of((pl_pcs), struct mdio_xpcs_args, pcs)
+
 static const int xpcs_usxgmii_features[] = {
 	ETHTOOL_LINK_MODE_Pause_BIT,
 	ETHTOOL_LINK_MODE_Asym_Pause_BIT,
@@ -385,7 +388,7 @@ static int xpcs_get_max_usxgmii_speed(const unsigned long *supported)
 	return max;
 }
 
-static int xpcs_config_usxgmii(struct mdio_xpcs_args *xpcs, int speed)
+static void xpcs_config_usxgmii(struct mdio_xpcs_args *xpcs, int speed)
 {
 	int ret, speed_sel;
 
@@ -410,33 +413,40 @@ static int xpcs_config_usxgmii(struct mdio_xpcs_args *xpcs, int speed)
 		break;
 	default:
 		/* Nothing to do here */
-		return -EINVAL;
+		return;
 	}
 
 	ret = xpcs_read_vpcs(xpcs, MDIO_CTRL1);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	ret = xpcs_write_vpcs(xpcs, MDIO_CTRL1, ret | DW_USXGMII_EN);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	ret = xpcs_read(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	ret &= ~DW_USXGMII_SS_MASK;
 	ret |= speed_sel | DW_USXGMII_FULL;
 
 	ret = xpcs_write(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1, ret);
 	if (ret < 0)
-		return ret;
+		goto out;
 
 	ret = xpcs_read_vpcs(xpcs, MDIO_CTRL1);
 	if (ret < 0)
-		return ret;
+		goto out;
+
+	ret = xpcs_write_vpcs(xpcs, MDIO_CTRL1, ret | DW_USXGMII_RST);
+	if (ret < 0)
+		goto out;
 
-	return xpcs_write_vpcs(xpcs, MDIO_CTRL1, ret | DW_USXGMII_RST);
+	return;
+
+out:
+	pr_err("%s: XPCS access returned %pe\n", __func__, ERR_PTR(ret));
 }
 
 static int _xpcs_config_aneg_c73(struct mdio_xpcs_args *xpcs)
@@ -765,14 +775,13 @@ static int xpcs_config_aneg_c37_sgmii(struct mdio_xpcs_args *xpcs)
 	return xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_DIG_CTRL1, ret);
 }
 
-static int xpcs_config(struct mdio_xpcs_args *xpcs,
-		       const struct phylink_link_state *state)
+static int xpcs_do_config(struct mdio_xpcs_args *xpcs, unsigned int mode)
 {
 	int ret;
 
 	switch (xpcs->an_mode) {
 	case DW_AN_C73:
-		if (state->an_enabled) {
+		if (phylink_autoneg_inband(mode)) {
 			ret = xpcs_config_aneg_c73(xpcs);
 			if (ret)
 				return ret;
@@ -790,6 +799,16 @@ static int xpcs_config(struct mdio_xpcs_args *xpcs,
 	return 0;
 }
 
+static int xpcs_config(struct phylink_pcs *pcs, unsigned int mode,
+		       phy_interface_t interface,
+		       const unsigned long *advertising,
+		       bool permit_pause_to_mac)
+{
+	struct mdio_xpcs_args *xpcs = phylink_pcs_to_xpcs(pcs);
+
+	return xpcs_do_config(xpcs, mode);
+}
+
 static int xpcs_get_state_c73(struct mdio_xpcs_args *xpcs,
 			      struct phylink_link_state *state)
 {
@@ -807,7 +826,7 @@ static int xpcs_get_state_c73(struct mdio_xpcs_args *xpcs,
 
 		state->link = 0;
 
-		return xpcs_config(xpcs, state);
+		return xpcs_do_config(xpcs, MLO_AN_INBAND);
 	}
 
 	if (state->an_enabled && xpcs_aneg_done_c73(xpcs, state)) {
@@ -864,36 +883,40 @@ static int xpcs_get_state_c37_sgmii(struct mdio_xpcs_args *xpcs,
 	return 0;
 }
 
-static int xpcs_get_state(struct mdio_xpcs_args *xpcs,
-			  struct phylink_link_state *state)
+static void xpcs_get_state(struct phylink_pcs *pcs,
+			   struct phylink_link_state *state)
 {
+	struct mdio_xpcs_args *xpcs = phylink_pcs_to_xpcs(pcs);
 	int ret;
 
 	switch (xpcs->an_mode) {
 	case DW_AN_C73:
 		ret = xpcs_get_state_c73(xpcs, state);
-		if (ret)
-			return ret;
+		if (ret) {
+			pr_err("xpcs_get_state_c73 returned %pe\n",
+			       ERR_PTR(ret));
+			return;
+		}
 		break;
 	case DW_AN_C37_SGMII:
 		ret = xpcs_get_state_c37_sgmii(xpcs, state);
-		if (ret)
-			return ret;
+		if (ret) {
+			pr_err("xpcs_get_state_c37_sgmii returned %pe\n",
+			       ERR_PTR(ret));
+		}
 		break;
 	default:
-		return -1;
+		return;
 	}
-
-	return 0;
 }
 
-static int xpcs_link_up(struct mdio_xpcs_args *xpcs, int speed,
-			phy_interface_t interface)
+static void xpcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
+			 phy_interface_t interface, int speed, int duplex)
 {
+	struct mdio_xpcs_args *xpcs = phylink_pcs_to_xpcs(pcs);
+
 	if (interface == PHY_INTERFACE_MODE_USXGMII)
 		return xpcs_config_usxgmii(xpcs, speed);
-
-	return 0;
 }
 
 static u32 xpcs_get_id(struct mdio_xpcs_args *xpcs)
@@ -934,6 +957,12 @@ static u32 xpcs_get_id(struct mdio_xpcs_args *xpcs)
 	return 0xffffffff;
 }
 
+static const struct phylink_pcs_ops xpcs_phylink_ops = {
+	.pcs_config = xpcs_config,
+	.pcs_get_state = xpcs_get_state,
+	.pcs_link_up = xpcs_link_up,
+};
+
 int xpcs_probe(struct mdio_xpcs_args *xpcs)
 {
 	u32 xpcs_id = xpcs_get_id(xpcs);
@@ -951,6 +980,9 @@ int xpcs_probe(struct mdio_xpcs_args *xpcs)
 		xpcs->id = entry;
 		xpcs->an_mode = entry->an_mode;
 
+		xpcs->pcs.ops = &xpcs_phylink_ops;
+		xpcs->pcs.poll = true;
+
 		return xpcs_soft_reset(xpcs);
 	}
 
@@ -958,16 +990,4 @@ int xpcs_probe(struct mdio_xpcs_args *xpcs)
 }
 EXPORT_SYMBOL_GPL(xpcs_probe);
 
-static struct mdio_xpcs_ops xpcs_ops = {
-	.config = xpcs_config,
-	.get_state = xpcs_get_state,
-	.link_up = xpcs_link_up,
-};
-
-struct mdio_xpcs_ops *mdio_xpcs_get_ops(void)
-{
-	return &xpcs_ops;
-}
-EXPORT_SYMBOL_GPL(mdio_xpcs_get_ops);
-
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index 11585fa093cd..eb74ab5b8138 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -18,22 +18,13 @@ struct xpcs_id;
 
 struct mdio_xpcs_args {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+	struct phylink_pcs pcs;
 	struct mii_bus *bus;
 	struct xpcs_id *id;
 	int addr;
 	int an_mode;
 };
 
-struct mdio_xpcs_ops {
-	int (*config)(struct mdio_xpcs_args *xpcs,
-		      const struct phylink_link_state *state);
-	int (*get_state)(struct mdio_xpcs_args *xpcs,
-			 struct phylink_link_state *state);
-	int (*link_up)(struct mdio_xpcs_args *xpcs, int speed,
-		       phy_interface_t interface);
-};
-
-struct mdio_xpcs_ops *mdio_xpcs_get_ops(void);
 void xpcs_validate(struct mdio_xpcs_args *xpcs, unsigned long *supported,
 		   struct phylink_link_state *state);
 int xpcs_config_eee(struct mdio_xpcs_args *xpcs, int mult_fact_100ns,
-- 
2.25.1

[RFC PATCH net-next 7/8] net: pcs: xpcs: use mdiobus_c45_addr in xpcs_{read,write}

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Use the dedicated helper for abstracting away how the clause 45 address
is packed in reg_addr.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/pcs/pcs-xpcs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 288abe8ddaf3..e0a7e546f32b 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -220,14 +220,14 @@ static struct xpcs_id {
 
 static int xpcs_read(struct mdio_xpcs_args *xpcs, int dev, u32 reg)
 {
-	u32 reg_addr = MII_ADDR_C45 | dev << 16 | reg;
+	u32 reg_addr = mdiobus_c45_addr(dev, reg);
 
 	return mdiobus_read(xpcs->bus, xpcs->addr, reg_addr);
 }
 
 static int xpcs_write(struct mdio_xpcs_args *xpcs, int dev, u32 reg, u16 val)
 {
-	u32 reg_addr = MII_ADDR_C45 | dev << 16 | reg;
+	u32 reg_addr = mdiobus_c45_addr(dev, reg);
 
 	return mdiobus_write(xpcs->bus, xpcs->addr, reg_addr, val);
 }
-- 
2.25.1

[RFC PATCH net-next 8/8] net: pcs: xpcs: convert to mdio_device

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Unify the 2 existing PCS drivers (lynx and xpcs) by doing a similar
thing on probe, which is to have a *_create function that takes a
struct mdio_device * given by the caller, and builds a private PCS
structure around that.

This changes stmmac to hold only a pointer to the xpcs, as opposed to
the full structure.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/stmicro/stmmac/common.h  |  2 +-
 .../ethernet/stmicro/stmmac/stmmac_ethtool.c  |  3 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  6 +--
 .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 32 +++++++--------
 drivers/net/pcs/pcs-xpcs.c                    | 40 ++++++++++++++-----
 include/linux/pcs/pcs-xpcs.h                  |  5 +--
 6 files changed, 54 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 678f8ce62b8a..8a83f9e1e95b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -503,7 +503,7 @@ struct mac_device_info {
 	const struct stmmac_hwtimestamp *ptp;
 	const struct stmmac_tc_ops *tc;
 	const struct stmmac_mmc_ops *mmc;
-	struct mdio_xpcs_args xpcs_args;
+	struct mdio_xpcs_args *xpcs;
 	struct mii_regs mii;	/* MII register Addresses */
 	struct mac_link link;
 	void __iomem *pcsr;     /* vpointer to device CSRs */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 72d2d575bbfe..bd5de43cf925 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -720,8 +720,7 @@ static int stmmac_ethtool_op_set_eee(struct net_device *dev,
 		netdev_warn(priv->dev,
 			    "Setting EEE tx-lpi is not supported\n");
 
-	ret = xpcs_config_eee(&priv->hw->xpcs_args,
-			      priv->plat->mult_fact_100ns,
+	ret = xpcs_config_eee(priv->hw->xpcs, priv->plat->mult_fact_100ns,
 			      edata->eee_enabled);
 	if (ret)
 		return ret;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3ccf00ea77d5..f46f6524aa18 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -996,7 +996,7 @@ static void stmmac_validate(struct phylink_config *config,
 	linkmode_andnot(state->advertising, state->advertising, mask);
 
 	/* If PCS is supported, check which modes it supports. */
-	xpcs_validate(&priv->hw->xpcs_args, supported, state);
+	xpcs_validate(priv->hw->xpcs, supported, state);
 }
 
 static void stmmac_fpe_link_state_handle(struct stmmac_priv *priv, bool is_up)
@@ -1222,7 +1222,7 @@ static int stmmac_phy_setup(struct stmmac_priv *priv)
 		return PTR_ERR(phylink);
 
 	if (mdio_bus_data->has_xpcs) {
-		struct mdio_xpcs_args *xpcs = &priv->hw->xpcs_args;
+		struct mdio_xpcs_args *xpcs = priv->hw->xpcs;
 
 		phylink_set_pcs(phylink, &xpcs->pcs);
 	}
@@ -3625,7 +3625,7 @@ int stmmac_open(struct net_device *dev)
 
 	if (priv->hw->pcs != STMMAC_PCS_TBI &&
 	    priv->hw->pcs != STMMAC_PCS_RTBI &&
-	    priv->hw->xpcs_args.an_mode != DW_AN_C73) {
+	    priv->hw->xpcs->an_mode != DW_AN_C73) {
 		ret = stmmac_init_phy(dev);
 		if (ret) {
 			netdev_err(priv->dev,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index 4a197b2fe26b..c632d3f10102 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -503,25 +503,25 @@ int stmmac_mdio_register(struct net_device *ndev)
 
 	/* Try to probe the XPCS by scanning all addresses. */
 	if (mdio_bus_data->has_xpcs) {
-		struct mdio_xpcs_args *xpcs = &priv->hw->xpcs_args;
-		int ret;
-
-		max_addr = PHY_MAX_ADDR;
-
-		xpcs->bus = new_bus;
-
-		found = 0;
-		for (addr = 0; addr < max_addr; addr++) {
-			xpcs->addr = addr;
-
-			ret = xpcs_probe(xpcs);
-			if (!ret) {
-				found = 1;
-				break;
+		struct mdio_device *mdiodev;
+		struct mdio_xpcs_args *xpcs;
+
+		for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
+			mdiodev = mdio_device_create(new_bus, addr);
+			if (IS_ERR(mdiodev))
+				continue;
+
+			xpcs = xpcs_create(mdiodev);
+			if (IS_ERR_OR_NULL(xpcs)) {
+				mdio_device_free(mdiodev);
+				continue;
 			}
+
+			priv->hw->xpcs = xpcs;
+			break;
 		}
 
-		if (!found && !mdio_node) {
+		if (!priv->hw->xpcs) {
 			dev_warn(dev, "No XPCS found\n");
 			err = -ENODEV;
 			goto no_xpcs_found;
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index e0a7e546f32b..194b79da547b 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -221,15 +221,19 @@ static struct xpcs_id {
 static int xpcs_read(struct mdio_xpcs_args *xpcs, int dev, u32 reg)
 {
 	u32 reg_addr = mdiobus_c45_addr(dev, reg);
+	struct mii_bus *bus = xpcs->mdiodev->bus;
+	int addr = xpcs->mdiodev->addr;
 
-	return mdiobus_read(xpcs->bus, xpcs->addr, reg_addr);
+	return mdiobus_read(bus, addr, reg_addr);
 }
 
 static int xpcs_write(struct mdio_xpcs_args *xpcs, int dev, u32 reg, u16 val)
 {
 	u32 reg_addr = mdiobus_c45_addr(dev, reg);
+	struct mii_bus *bus = xpcs->mdiodev->bus;
+	int addr = xpcs->mdiodev->addr;
 
-	return mdiobus_write(xpcs->bus, xpcs->addr, reg_addr, val);
+	return mdiobus_write(bus, addr, reg_addr, val);
 }
 
 static int xpcs_read_vendor(struct mdio_xpcs_args *xpcs, int dev, u32 reg)
@@ -294,7 +298,7 @@ static int xpcs_soft_reset(struct mdio_xpcs_args *xpcs)
 #define xpcs_warn(__xpcs, __state, __args...) \
 ({ \
 	if ((__state)->link) \
-		dev_warn(&(__xpcs)->bus->dev, ##__args); \
+		dev_warn(&(__xpcs)->mdiodev->dev, ##__args); \
 })
 
 static int xpcs_read_fault_c73(struct mdio_xpcs_args *xpcs,
@@ -963,10 +967,19 @@ static const struct phylink_pcs_ops xpcs_phylink_ops = {
 	.pcs_link_up = xpcs_link_up,
 };
 
-int xpcs_probe(struct mdio_xpcs_args *xpcs)
+struct mdio_xpcs_args *xpcs_create(struct mdio_device *mdiodev)
 {
-	u32 xpcs_id = xpcs_get_id(xpcs);
-	int i;
+	struct mdio_xpcs_args *xpcs;
+	u32 xpcs_id;
+	int i, ret;
+
+	xpcs = kzalloc(sizeof(*xpcs), GFP_KERNEL);
+	if (!xpcs)
+		return NULL;
+
+	xpcs->mdiodev = mdiodev;
+
+	xpcs_id = xpcs_get_id(xpcs);
 
 	for (i = 0; i < ARRAY_SIZE(xpcs_id_list); i++) {
 		struct xpcs_id *entry = &xpcs_id_list[i];
@@ -983,11 +996,20 @@ int xpcs_probe(struct mdio_xpcs_args *xpcs)
 		xpcs->pcs.ops = &xpcs_phylink_ops;
 		xpcs->pcs.poll = true;
 
-		return xpcs_soft_reset(xpcs);
+		ret = xpcs_soft_reset(xpcs);
+		if (ret)
+			goto out;
+
+		return xpcs;
 	}
 
-	return -ENODEV;
+	ret = -ENODEV;
+
+out:
+	kfree(xpcs);
+
+	return ERR_PTR(ret);
 }
-EXPORT_SYMBOL_GPL(xpcs_probe);
+EXPORT_SYMBOL_GPL(xpcs_create);
 
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index eb74ab5b8138..237f2198f709 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -18,10 +18,9 @@ struct xpcs_id;
 
 struct mdio_xpcs_args {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
+	struct mdio_device *mdiodev;
 	struct phylink_pcs pcs;
-	struct mii_bus *bus;
 	struct xpcs_id *id;
-	int addr;
 	int an_mode;
 };
 
@@ -29,6 +28,6 @@ void xpcs_validate(struct mdio_xpcs_args *xpcs, unsigned long *supported,
 		   struct phylink_link_state *state);
 int xpcs_config_eee(struct mdio_xpcs_args *xpcs, int mult_fact_100ns,
 		    int enable);
-int xpcs_probe(struct mdio_xpcs_args *xpcs);
+struct mdio_xpcs_args *xpcs_create(struct mdio_device *mdiodev);
 
 #endif /* __LINUX_PCS_XPCS_H */
-- 
2.25.1

Re: [RFC PATCH net-next 0/8] Convert xpcs to phylink_pcs_ops

[Wong Vee Khee]

On Thu, May 27, 2021 at 11:45:20PM +0300, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This patch series is COMPLETELY UNTESTED (I don't have stmmac hardware
> with the xpcs) hence the RFC tag. If people from Intel could test this
> it would be great.
> 
> Background: the sja1105 DSA driver currently drives a Designware XPCS
> for SGMII and 2500base-X, and it would be nice to reuse some code with
> the xpcs module. This would also help consolidate the phylink_pcs_ops,
> since the only user of that, currently, is the lynx_pcs.
> 
> Therefore, this series makes the xpcs expose the same kind of API that
> the lynx_pcs module does.
> 
> Note: this patch series must be applied on top of:
> https://patchwork.kernel.org/project/netdevbpf/patch/20210527155959.3270478-1-olteanv@gmail.com/
> 
> Vladimir Oltean (8):
>   net: pcs: xpcs: delete shim definition for mdio_xpcs_get_ops()
>   net: pcs: xpcs: check for supported PHY interface modes in
>     phylink_validate
>   net: pcs: xpcs: export xpcs_validate
>   net: pcs: export xpcs_config_eee
>   net: pcs: xpcs: export xpcs_probe
>   net: pcs: xpcs: convert to phylink_pcs_ops
>   net: pcs: xpcs: use mdiobus_c45_addr in xpcs_{read,write}
>   net: pcs: xpcs: convert to mdio_device
> 
>  drivers/net/ethernet/stmicro/stmmac/common.h  |   3 +-
>  drivers/net/ethernet/stmicro/stmmac/hwif.h    |  14 --
>  .../ethernet/stmicro/stmmac/stmmac_ethtool.c  |   5 +-
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c |  41 +---
>  .../net/ethernet/stmicro/stmmac/stmmac_mdio.c |  41 ++--
>  drivers/net/pcs/pcs-xpcs.c                    | 199 +++++++++++-------
>  include/linux/pcs/pcs-xpcs.h                  |  35 +--
>  7 files changed, 162 insertions(+), 176 deletions(-)

I got the following kernel panic after applying [1], and followed by
this patch series.

[1] https://patchwork.kernel.org/project/netdevbpf/patch/20210527155959.3270478-1-olteanv@gmail.com/


[   10.742057] libphy: stmmac: probed
[   10.750396] mdio_bus stmmac-1:01: attached PHY driver [unbound] (mii_bus:phy_addr=stmmac-1:01, irq=POLL)
[   10.818222] intel-eth-pci 0000:00:1e.4 (unnamed net_device) (uninitialized): failed to validate link configuration for in-band status
[   10.830348] intel-eth-pci 0000:00:1e.4 (unnamed net_device) (uninitialized): failed to setup phy (-22)
[   10.879931] ish-hid {33AECD58-B679-4E54-9BD9-A04D34F0C226}: [hid-ish]: enum_devices_done OK, num_hid_devices=6
[   10.901311] hid-generic 001F:8087:0AC2.0001: device has no listeners, quitting
[   10.922498] hid-generic 001F:8087:0AC2.0002: device has no listeners, quitting
[   10.940073] hid-generic 001F:8087:0AC2.0003: device has no listeners, quitting
[   10.951878] hid-generic 001F:8087:0AC2.0004: device has no listeners, quitting
[   10.958230] atkbd serio0: Failed to enable keyboard on isa0060/serio0
[   10.965799] input: AT Translated Set 2 keyboard as /devices/platform/i8042/serio0/input/input5
[   10.978620] ------------[ cut here ]------------
[   10.983295] stmmac-0000:00:1e.4 already disabled
[   10.987973] WARNING: CPU: 3 PID: 2588 at drivers/clk/clk.c:952 clk_core_disable+0x96/0x1b0
[   10.989541] hid-generic 001F:8087:0AC3.0005: device has no listeners, quitting
[   10.996322] Modules linked in: intel_ishtp_hid(+) ax88179_178a dwmac_intel(+) usbnet x86_pkg_temp_thermal stmmac mii kvm_intel dwc3 atkbd mei_wdt udc_core pcs_xpcs libps2 mei_hdcp kvm phylink libphy irqbypass spi_pxa2xx_platform i915(+) wdat_wdt dw_dmac mei_me dw_dmac_core i2c_i801 intel_ish_ipc intel_rapl_msr intel_ishtp pcspkr i2c_smbus mei dwc3_pci thermal tpm_crb tpm_tis tpm_tis_core parport_pc parport tpm i8042 intel_pmc_core sch_fq_codel uhid fuse configfs snd_hda_intel snd_intel_dspcfg snd_intel_sdw_acpi snd_hda_codec snd_hda_core snd_pcm snd_timer snd soundcore
[   11.006487] hid-generic 001F:8087:0AC3.0006: device has no listeners, quitting
[   11.054376] CPU: 3 PID: 2588 Comm: systemd-udevd Tainted: G     U            5.13.0-rc3-intel-lts #68
[   11.054378] Hardware name: Intel Corporation Tiger Lake Client Platform/TigerLake U DDR4 SODIMM RVP, BIOS TGLIFUI1.R00.3373.AF0.2009230546 09/23/2020
[   11.084467] RIP: 0010:clk_core_disable+0x96/0x1b0
[   11.089221] Code: 00 8b 05 45 96 17 01 85 c0 7f 24 48 8b 5b 30 48 85 db 74 a5 8b 43 7c 85 c0 75 93 48 8b 33 48 c7 c7 6e 32 0c 8d e8 b2 5d 52 00 <0f> 0b 5b 5d c3 65 8b 05 76 31 d8 73 89 c0 48 0f a3 05 bc 92 1a 01
[   11.108121] RSP: 0018:ffffacc50038baa0 EFLAGS: 00010086
[   11.113400] RAX: 0000000000000000 RBX: ffff9702895e3800 RCX: 0000000000000000
[   11.120594] RDX: 0000000000000002 RSI: ffffffff8d062d5f RDI: 00000000ffffffff
[   11.127794] RBP: 0000000000000283 R08: 0000000000000000 R09: ffffacc50038b8d0
[   11.134995] R10: 0000000000000001 R11: 0000000000000001 R12: ffff9702895e3800
[   11.142189] R13: 0000000000000006 R14: ffff970282a960c8 R15: ffffacc50038bad0
[   11.149382] FS:  00007fe7654f6780(0000) GS:ffff970417f80000(0000) knlGS:0000000000000000
[   11.157536] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   11.163337] CR2: 00007f5e93783000 CR3: 0000000108ca0001 CR4: 0000000000770ee0
[   11.170530] PKRU: 55555554
[   11.173275] Call Trace:
[   11.175755]  clk_core_disable_lock+0x1b/0x30
[   11.180075]  intel_eth_pci_probe.cold+0x11d/0x136 [dwmac_intel]
[   11.186055]  pci_device_probe+0xcf/0x150
[   11.190021]  really_probe+0xf5/0x3e0
[   11.193646]  driver_probe_device+0x64/0x150
[   11.197874]  device_driver_attach+0x53/0x60
[   11.202103]  __driver_attach+0x9f/0x150
[   11.205984]  ? device_driver_attach+0x60/0x60
[   11.210387]  ? device_driver_attach+0x60/0x60
[   11.214789]  bus_for_each_dev+0x77/0xc0
[   11.218670]  bus_add_driver+0x184/0x1f0
[   11.222550]  driver_register+0x6c/0xc0
[   11.226347]  ? 0xffffffffc0641000
[   11.229705]  do_one_initcall+0x4a/0x210
[   11.233585]  ? kmem_cache_alloc_trace+0x305/0x4e0
[   11.238337]  do_init_module+0x5c/0x230
[   11.242127]  load_module+0x2894/0x2b70
[   11.245919]  ? __do_sys_finit_module+0xb5/0x120
[   11.250496]  __do_sys_finit_module+0xb5/0x120
[   11.254899]  do_syscall_64+0x42/0x80
[   11.258517]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[   11.263624] RIP: 0033:0x7fe76579bd4d
[   11.267247] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 03 31 0c 00 f7 d8 64 89 01 48
[   11.286138] RSP: 002b:00007ffd7aaa2e08 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[   11.293772] RAX: ffffffffffffffda RBX: 00005649a8ea6990 RCX: 00007fe76579bd4d
[   11.300964] RDX: 0000000000000000 RSI: 00007fe76592f1e3 RDI: 0000000000000012
[   11.308158] RBP: 0000000000020000 R08: 0000000000000000 R09: 0000000000000000
[   11.315351] R10: 0000000000000012 R11: 0000000000000246 R12: 00007fe76592f1e3
[   11.322544] R13: 0000000000000000 R14: 0000000000000000 R15: 00007ffd7aaa2fc8
[   11.329739] ---[ end trace 2cfbe6d1617011ac ]---
[   11.334825] ------------[ cut here ]------------


HTH,
 VK

Re: [RFC PATCH net-next 0/8] Convert xpcs to phylink_pcs_ops

[Vladimir Oltean]

Hi VK,

On Fri, May 28, 2021 at 10:15:21AM +0800, Wong Vee Khee wrote:
> I got the following kernel panic after applying [1], and followed by
> this patch series.
> 
> [1] https://patchwork.kernel.org/project/netdevbpf/patch/20210527155959.3270478-1-olteanv@gmail.com/
> 
> [   10.742057] libphy: stmmac: probed
> [   10.750396] mdio_bus stmmac-1:01: attached PHY driver [unbound] (mii_bus:phy_addr=stmmac-1:01, irq=POLL)
> [   10.818222] intel-eth-pci 0000:00:1e.4 (unnamed net_device) (uninitialized): failed to validate link configuration for in-band status
> [   10.830348] intel-eth-pci 0000:00:1e.4 (unnamed net_device) (uninitialized): failed to setup phy (-22)

Thanks a lot for testing. Sadly I can't figure out what is the mistake.
Could you please add this debugging patch on top and let me know what it
prints?

-----------------------------[ cut here ]-----------------------------
From 1d745a51b53b38df432a33849632a1b553d3f90a Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Fri, 28 May 2021 12:00:17 +0300
Subject: [PATCH] xpcs debug

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/pcs/pcs-xpcs.c | 44 +++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 194b79da547b..4268b8bb8db0 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -675,30 +675,39 @@ static void xpcs_resolve_pma(struct mdio_xpcs_args *xpcs,
 void xpcs_validate(struct mdio_xpcs_args *xpcs, unsigned long *supported,
 		   struct phylink_link_state *state)
 {
-	bool valid_interface;
-
-	if (state->interface == PHY_INTERFACE_MODE_NA) {
-		valid_interface = true;
-	} else {
+	if (state->interface != PHY_INTERFACE_MODE_NA) {
 		struct xpcs_id *id = xpcs->id;
+		bool valid_interface = false;
 		int i;
 
-		valid_interface = false;
-
 		for (i = 0; id->interface[i] != PHY_INTERFACE_MODE_MAX; i++) {
-			if (id->interface[i] != state->interface)
+			if (id->interface[i] != state->interface) {
+				dev_err(&xpcs->mdiodev->dev,
+					"%s: provided interface %s does not match supported interface %d (%s)\n",
+					__func__, phy_modes(state->interface),
+					i, phy_modes(id->interface[i]));
 				continue;
+			}
 
 			valid_interface = true;
 			break;
 		}
-	}
 
-	if (!valid_interface) {
-		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
-		return;
+		if (!valid_interface) {
+			dev_err(&xpcs->mdiodev->dev,
+				"%s: provided interface %s does not match any supported interface\n",
+				__func__, phy_modes(state->interface));
+			bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+			return;
+		}
 	}
 
+	dev_err(&xpcs->mdiodev->dev,
+		"%s: supported mask for interface %s is %*pb, received supported mask is %*pb\n",
+		__func__, phy_modes(state->interface),
+		__ETHTOOL_LINK_MODE_MASK_NBITS, xpcs->supported,
+		__ETHTOOL_LINK_MODE_MASK_NBITS, supported);
+
 	linkmode_and(supported, supported, xpcs->supported);
 	linkmode_and(state->advertising, state->advertising, xpcs->supported);
 }
@@ -987,8 +996,17 @@ struct mdio_xpcs_args *xpcs_create(struct mdio_device *mdiodev)
 		if ((xpcs_id & entry->mask) != entry->id)
 			continue;
 
-		for (i = 0; entry->supported[i] != __ETHTOOL_LINK_MODE_MASK_NBITS; i++)
+		dev_err(&mdiodev->dev, "%s: xpcs_id %x matched on entry %d\n",
+			__func__, xpcs_id, i);
+
+		for (i = 0; entry->supported[i] != __ETHTOOL_LINK_MODE_MASK_NBITS; i++) {
+			dev_err(&mdiodev->dev, "%s: setting entry->supported bit %d\n",
+				__func__, entry->supported[i]);
 			set_bit(entry->supported[i], xpcs->supported);
+		}
+
+		dev_err(&mdiodev->dev, "%s: xpcs->supported %*pb\n", __func__,
+			__ETHTOOL_LINK_MODE_MASK_NBITS, xpcs->supported);
 
 		xpcs->id = entry;
 		xpcs->an_mode = entry->an_mode;
-----------------------------[ cut here ]-----------------------------

Re: [RFC PATCH net-next 0/8] Convert xpcs to phylink_pcs_ops

[Wong Vee Khee]

On Fri, May 28, 2021 at 12:12:30PM +0300, Vladimir Oltean wrote:
> Hi VK,
> 
> On Fri, May 28, 2021 at 10:15:21AM +0800, Wong Vee Khee wrote:
> > I got the following kernel panic after applying [1], and followed by
> > this patch series.
> > 
> > [1] https://patchwork.kernel.org/project/netdevbpf/patch/20210527155959.3270478-1-olteanv@gmail.com/
> > 
> > [   10.742057] libphy: stmmac: probed
> > [   10.750396] mdio_bus stmmac-1:01: attached PHY driver [unbound] (mii_bus:phy_addr=stmmac-1:01, irq=POLL)
> > [   10.818222] intel-eth-pci 0000:00:1e.4 (unnamed net_device) (uninitialized): failed to validate link configuration for in-band status
> > [   10.830348] intel-eth-pci 0000:00:1e.4 (unnamed net_device) (uninitialized): failed to setup phy (-22)
> 
> Thanks a lot for testing. Sadly I can't figure out what is the mistake.
> Could you please add this debugging patch on top and let me know what it
> prints?
> 

Sorry for the late response. Here the debug log:

[   11.474302] mdio_bus stmmac-1:01: attached PHY driver [unbound] (mii_bus:phy_addr=stmmac-1:01, irq=POLL)
[   11.495564] mdio_bus stmmac-1:16: xpcs_create: xpcs_id 7996ced0 matched on entry 0
[   11.503154] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 13
[   11.510377] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 14
[   11.517590] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 6
[   11.524725] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 17
[   11.531946] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 18
[   11.539278] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 19
[   11.541316] ish-hid {33AECD58-B679-4E54-9BD9-A04D34F0C226}: [hid-ish]: enum_devices_done OK, num_hid_devices=6
[   11.546487] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 15
[   11.546489] mdio_bus stmmac-1:16: xpcs_create: xpcs->supported 0000000,00000000,000ee040
[   11.584687] hid-generic 001F:8087:0AC2.0001: device has no listeners, quitting
[   11.599461] mdio_bus stmmac-1:16: xpcs_validate: provided interface sgmii does not match supported interface 0 (usxgmii)
[   11.606538] hid-generic 001F:8087:0AC2.0002: device has no listeners, quitting
[   11.610306] mdio_bus stmmac-1:16: xpcs_validate: provided interface sgmii does not match any supported interface
[   11.610309] mdio_bus stmmac-1:16: xpcs_validate: provided interface sgmii does not match supported interface 0 (usxgmii)
[   11.626259] hid-generic 001F:8087:0AC2.0003: device has no listeners, quitting
[   11.627675] mdio_bus stmmac-1:16: xpcs_validate: provided interface sgmii does not match any supported interface
[   11.627677] intel-eth-pci 0000:00:1e.4 (unnamed net_device) (uninitialized): failed to validate link configuration for in-band status
[   11.641996] hid-generic 001F:8087:0AC2.0004: device has no listeners, quitting
[   11.645729] intel-eth-pci 0000:00:1e.4 (unnamed net_device) (uninitialized): failed to setup phy (-22)

Regards,
VK

Re: [RFC PATCH net-next 0/8] Convert xpcs to phylink_pcs_ops

[Vladimir Oltean]

Hi VK,

On Mon, May 31, 2021 at 10:30:19AM +0800, Wong Vee Khee wrote:
> On Fri, May 28, 2021 at 12:12:30PM +0300, Vladimir Oltean wrote:
> > Hi VK,
> > 
> > On Fri, May 28, 2021 at 10:15:21AM +0800, Wong Vee Khee wrote:
> > > I got the following kernel panic after applying [1], and followed by
> > > this patch series.
> > > 
> > > [1] https://patchwork.kernel.org/project/netdevbpf/patch/20210527155959.3270478-1-olteanv@gmail.com/
> > > 
> > > [   10.742057] libphy: stmmac: probed
> > > [   10.750396] mdio_bus stmmac-1:01: attached PHY driver [unbound] (mii_bus:phy_addr=stmmac-1:01, irq=POLL)
> > > [   10.818222] intel-eth-pci 0000:00:1e.4 (unnamed net_device) (uninitialized): failed to validate link configuration for in-band status
> > > [   10.830348] intel-eth-pci 0000:00:1e.4 (unnamed net_device) (uninitialized): failed to setup phy (-22)
> > 
> > Thanks a lot for testing. Sadly I can't figure out what is the mistake.
> > Could you please add this debugging patch on top and let me know what it
> > prints?
> > 
> 
> Sorry for the late response. Here the debug log:
> 
> [   11.474302] mdio_bus stmmac-1:01: attached PHY driver [unbound] (mii_bus:phy_addr=stmmac-1:01, irq=POLL)
> [   11.495564] mdio_bus stmmac-1:16: xpcs_create: xpcs_id 7996ced0 matched on entry 0
> [   11.503154] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 13
> [   11.510377] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 14
> [   11.517590] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 6
> [   11.524725] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 17
> [   11.531946] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 18
> [   11.539278] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 19
> [   11.541316] ish-hid {33AECD58-B679-4E54-9BD9-A04D34F0C226}: [hid-ish]: enum_devices_done OK, num_hid_devices=6
> [   11.546487] mdio_bus stmmac-1:16: xpcs_create: setting entry->supported bit 15
> [   11.546489] mdio_bus stmmac-1:16: xpcs_create: xpcs->supported 0000000,00000000,000ee040
> [   11.584687] hid-generic 001F:8087:0AC2.0001: device has no listeners, quitting
> [   11.599461] mdio_bus stmmac-1:16: xpcs_validate: provided interface sgmii does not match supported interface 0 (usxgmii)
> [   11.606538] hid-generic 001F:8087:0AC2.0002: device has no listeners, quitting
> [   11.610306] mdio_bus stmmac-1:16: xpcs_validate: provided interface sgmii does not match any supported interface
> [   11.610309] mdio_bus stmmac-1:16: xpcs_validate: provided interface sgmii does not match supported interface 0 (usxgmii)
> [   11.626259] hid-generic 001F:8087:0AC2.0003: device has no listeners, quitting
> [   11.627675] mdio_bus stmmac-1:16: xpcs_validate: provided interface sgmii does not match any supported interface
> [   11.627677] intel-eth-pci 0000:00:1e.4 (unnamed net_device) (uninitialized): failed to validate link configuration for in-band status
> [   11.641996] hid-generic 001F:8087:0AC2.0004: device has no listeners, quitting
> [   11.645729] intel-eth-pci 0000:00:1e.4 (unnamed net_device) (uninitialized): failed to setup phy (-22)

Ha ha, this works as expected, but I was led into error due to the code
structure.

See, everything in pcs-xpcs.c is laid out as if there are different PHY
IDs for SGMII, USXGMII etc. But if you pay close attention, they are all
equal to 0x7996ced0:

#define SYNOPSYS_XPCS_USXGMII_ID	0x7996ced0
#define SYNOPSYS_XPCS_10GKR_ID		0x7996ced0
#define SYNOPSYS_XPCS_XLGMII_ID		0x7996ced0
#define SYNOPSYS_XPCS_SGMII_ID		0x7996ced0
#define SYNOPSYS_XPCS_MASK		0xffffffff

With the old code, it works because the probing code gets a nudge from
the caller of xpcs_probe by being told what is the expected phy_interface_t.
The xpcs then uses the phy_interface_t _as_part_of_ the PHY ID matching
sequence.

So.. yeah. I got the information I needed. I will come back with a way
for the same PCS PHY ID to support multiple PHY interface types.

Thanks again for testing.