Re: [RFC PATCH v2 net-next 9/9] net: pcs: xpcs: convert to phylink_pcs_ops

On Tue, Jun 01, 2021 at 01:10:33PM +0100, Russell King (Oracle) wrote:

quoted

On Tue, Jun 01, 2021 at 03:33:25AM +0300, Vladimir Oltean wrote:

quoted

 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_config is still a required function.

This is correct, thanks.

VK, would you mind testing again with this extra patch added to the mix?
If it works, I will add it to the series in v3, ordered properly.

-----------------------------[ cut here]-----------------------------
From a79863027998451c73d5bbfaf1b77cf6097a110c Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Wed, 2 Jun 2021 16:35:55 +0300
Subject: [PATCH] net: phylink: allow the mac_config method to be missing if
 pcs_ops are provided

The pcs_config method from struct phylink_pcs_ops does everything that
the mac_config method from struct phylink_mac_ops used to do in the
legacy approach of driving a MAC PCS. So allow drivers to not implement
the mac_config method if there is nothing to do. Keep the method
required for setups that do not provide pcs_ops.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/phylink.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 96d8e88b4e46..a8842c6ce3a2 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c

@@ -415,6 +415,9 @@ static void phylink_resolve_flow(struct phylink_link_state *state)
 static void phylink_mac_config(struct phylink *pl,
 			       const struct phylink_link_state *state)
 {
+	if (!pl->mac_ops->mac_config)
+		return;
+
 	phylink_dbg(pl,
 		    "%s: mode=%s/%s/%s/%s adv=%*pb pause=%02x link=%u an=%u\n",
 		    __func__, phylink_an_mode_str(pl->cur_link_an_mode),

@@ -1192,6 +1195,12 @@ void phylink_start(struct phylink *pl)
 
 	ASSERT_RTNL();
 
+	/* The mac_ops::mac_config method may be absent only if the
+	 * pcs_ops are present.
+	 */
+	if (WARN_ON_ONCE(!pl->mac_ops->mac_config && !pl->pcs_ops))
+		return;
+
 	phylink_info(pl, "configuring for %s/%s link mode\n",
 		     phylink_an_mode_str(pl->cur_link_an_mode),
 		     phy_modes(pl->link_config.interface));