net: stmmac: provide platform select_pcs method

Hi,

This series adds a select_pcs() method to the stmmac platform data to allow platforms that need to provide their own PCSes to do so, moving the decision making into platform code.

This avoids questions such as "what should the priority of XPCS vs some other platform PCS be?" and when we provide a PCS for the internal PCS, how that interacts with both the XPCS and platform provided PCS.

Note that if a platform implements the select_pcs() method, then the return values are:
- a phylink_pcs pointer - the PCS to be used.
- NULL - no phylink_pcs to be used.
Otherwise (if not implemented or returns an error-pointer), then allow the the stmmac internal PCS to be used if appropriate (once that patch set is merged.)

Patch 1 introduces the new method.
Patch 2 converts Intel mGBE to use this to provide the XPCS and
removes the XPCS decision making from core code.
Patch 3 provides an implementation for rzn1 to return its PCS. Patch 4 does the same for socfpga.
Patch 5 removes the core code returning priv->hw->phylink_pcs.

No functional change is anticipated. Once this has been merged, it will be expected that platforms should populate all three PCS methods or none of the PCS methods.

v1->v2:
- Fix build error in patch 2.
- Add attributations from Romain Gantois.

 drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c   | 11 +++++++++++
 drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c    |  7 +++++++
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c |  7 +++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c   | 10 +++++++---
 include/linux/stmmac.h                              |  4 +++-
 5 files changed, 35 insertions(+), 4 deletions(-)

RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Allow platform drivers to provide their logic to select an appropriate PCS.

---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 +++++++
 include/linux/stmmac.h                            | 4 +++-
 2 files changed, 10 insertions(+), 1 deletion(-)

index bbedf2a8c60f..302aa4080de3 100644
@@ -949,6 +949,13 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,
 						 phy_interface_t interface)
 {
 	struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
+	struct phylink_pcs *pcs;
+
+	if (priv->plat->select_pcs) {
+		pcs = priv->plat->select_pcs(priv, interface);
+		if (!IS_ERR(pcs))
+			return pcs;
+	}
 
 	if (priv->hw->xpcs)
 		return &priv->hw->xpcs->pcs;

index 8f0f156d50d3..9c54f82901a1 100644
@@ -13,7 +13,7 @@
 #define __STMMAC_PLATFORM_DATA
 
 #include <linux/platform_device.h>
-#include <linux/phy.h>
+#include <linux/phylink.h>
 
 #define MTL_MAX_RX_QUEUES	8
 #define MTL_MAX_TX_QUEUES	8

@@ -271,6 +271,8 @@ struct plat_stmmacenet_data {
 	void (*dump_debug_regs)(void *priv);
 	int (*pcs_init)(struct stmmac_priv *priv);
 	void (*pcs_exit)(struct stmmac_priv *priv);
+	struct phylink_pcs *(*select_pcs)(struct stmmac_priv *priv,
+					  phy_interface_t interface);
 	void *bsp_priv;
 	struct clk *stmmac_clk;
 	struct clk *pclk;

2.30.2

Hi Russell

From phylink's perspective, it's not part of the PCS, it's something that the MAC does.

The interface passed in to mac_select_pcs() so so the MAC code can decide which PCS (if it has many to choose from) will be used for the specified interface mode to either a PHY or other device connected to the netdev. It isn't a PCS operation, it's an operation that returns an appropriate PCS.

So, I want to keep "select_pcs" as at least a suffix, because it is selecting a PCS. Eventually, I would like to see the stmmac implementations check the "interface" passed to it before deciding whether to return a PCS or not - thus how it's intended to be implemented.

"pcs_select" seems to make it sound like it's part of a PCS implementation, which as I've explained above, it isn't. It also doesn't convey that it's selecting a PCS based on its arguments.

I'd also like to keep the ability to grep for "select_pcs" to find implementations and not have complex grep expressions to find whatever the driver has called it! To that end, I much prefer that drivers that name sub-implementations the same way that phylink names them to make grepping easier.

RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Move the code returning the XPCS into dwmac-intel, which is the only user of XPCS. Fill in the select_pcs() implementation only when we are going to setup the XPCS, thus when it should be present.

---
 drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 11 +++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  3 ---
 2 files changed, 11 insertions(+), 3 deletions(-)

index 56649edb18cd..094d34c4193c 100644
@@ -443,6 +443,16 @@ static void common_default_data(struct plat_stmmacenet_data *plat)
 	plat->rx_queues_cfg[0].pkt_route = 0x0;
 }
 
+static struct phylink_pcs *intel_mgbe_select_pcs(struct stmmac_priv *priv,
+						 phy_interface_t interface)
+{
+	/* plat->mdio_bus_data->has_xpcs has been set true, so there
+	 * should always be an XPCS. The original code would always
+	 * return this if present.
+	 */
+	return &priv->hw->xpcs->pcs;
+}
+
 static int intel_mgbe_common_data(struct pci_dev *pdev,
 				  struct plat_stmmacenet_data *plat)
 {

@@ -587,6 +597,7 @@ static int intel_mgbe_common_data(struct pci_dev *pdev,
 	    plat->phy_interface == PHY_INTERFACE_MODE_1000BASEX) {
 		plat->mdio_bus_data->has_xpcs = true;
 		plat->mdio_bus_data->default_an_inband = true;
+		plat->select_pcs = intel_mgbe_select_pcs;
 	}
 
 	/* Ensure mdio bus scan skips intel serdes and pcs-xpcs */

index 302aa4080de3..e9e2a95c91a3 100644
@@ -957,9 +957,6 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,
 			return pcs;
 	}
 
-	if (priv->hw->xpcs)
-		return &priv->hw->xpcs->pcs;
-
 	return priv->hw->phylink_pcs;
 }
 

2.30.2

Provide a .select_pcs() implementation which returns the phylink PCS that was created in the .pcs_init() method.

---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c | 7 +++++++
 1 file changed, 7 insertions(+)

index 848cf3c01f4a..59a7bd560f96 100644
@@ -39,6 +39,12 @@ static void rzn1_dwmac_pcs_exit(struct stmmac_priv *priv)
 		miic_destroy(priv->hw->phylink_pcs);
 }
 
+static struct phylink_pcs *rzn1_dwmac_select_pcs(struct stmmac_priv *priv,
+						 phy_interface_t interface)
+{
+	return priv->hw->phylink_pcs;
+}
+
 static int rzn1_dwmac_probe(struct platform_device *pdev)
 {
 	struct plat_stmmacenet_data *plat_dat;

@@ -57,6 +63,7 @@ static int rzn1_dwmac_probe(struct platform_device *pdev)
 	plat_dat->bsp_priv = plat_dat;
 	plat_dat->pcs_init = rzn1_dwmac_pcs_init;
 	plat_dat->pcs_exit = rzn1_dwmac_pcs_exit;
+	plat_dat->select_pcs = rzn1_dwmac_select_pcs;
 
 	ret = stmmac_dvr_probe(dev, plat_dat, &stmmac_res);
 	if (ret)

2.30.2

Provide a .select_pcs() implementation which returns the phylink PCS that was created in the .pcs_init() method.

---
 drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 7 +++++++
 1 file changed, 7 insertions(+)

index b3d45f9dfb55..fdb4c773ec98 100644
@@ -429,6 +429,12 @@ static void socfpga_dwmac_pcs_exit(struct stmmac_priv *priv)
 		lynx_pcs_destroy(priv->hw->phylink_pcs);
 }
 
+static struct phylink_pcs *socfpga_dwmac_select_pcs(struct stmmac_priv *priv,
+						    phy_interface_t interface)
+{
+	return priv->hw->phylink_pcs;
+}
+
 static int socfpga_dwmac_probe(struct platform_device *pdev)
 {
 	struct plat_stmmacenet_data *plat_dat;

@@ -478,6 +484,7 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
 	plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
 	plat_dat->pcs_init = socfpga_dwmac_pcs_init;
 	plat_dat->pcs_exit = socfpga_dwmac_pcs_exit;
+	plat_dat->select_pcs = socfpga_dwmac_select_pcs;
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 	if (ret)

2.30.2

Since all platform providers of PCS now populate the select_pcs() method, there is no need for the common code to look at priv->hw->phylink_pcs, so remove it.

---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index e9e2a95c91a3..5ddbb0d44373 100644
@@ -957,7 +957,7 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,
 			return pcs;
 	}
 
-	return priv->hw->phylink_pcs;
+	return NULL;
 }
 
 static void stmmac_mac_config(struct phylink_config *config, unsigned int mode,

2.30.2