[PATCH net-next 0/4] Remove legacy phylink behaviour

[PATCH net-next 0/4] Remove legacy phylink behaviour

[Russell King (Oracle)]

Hi,

This series removes the - as far as I can tell - unreachable code in
mtk_eth_soc that relies upon legacy phylink behaviour, and then removes
the support in phylink for this legacy behaviour.

Patch 1 removes the clocking configuration from mtk_eth_soc for non-
TRGMII, non-serdes based interface modes, and disables those interface
modes prior to phylink configuration.

Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which
I believe is also not used - mtk_eth_soc appears not to be used with
SFPs (which would use a kind of in-band mode) nor does any DT appear
to specify in-band mode for any non-serdes based interface mode.

With both of those dealt with, the kernel is now free of any driver
relying on the phylink legacy mode. Therefore, patch 3 removes support
for this.

Finally, with the advent of a new driver being submitted today that
makes use of state->speed in the mac_config() path, patch 4 ensures that
any phylink_link_state member that should not be used in mac_config is
either cleared or set to an invalid value.

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 94 +++++------------------------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  1 +
 drivers/net/phy/phylink.c                   | 48 ++++++---------
 include/linux/phylink.h                     | 45 ++------------
 4 files changed, 42 insertions(+), 146 deletions(-)
-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[PATCH net-next 1/4] net: ethernet: mtk_eth_soc: remove incorrect PLL configuration

[Russell King (Oracle)]

MT7623 GMAC0 attempts to configure the system clocking according to the
required speed in the .mac_config callback for non-SGMII, non-baseX and
non-TRGMII modes.

state->speed setting has never been reliable in the .mac_config
callback - there are cases where this is not the link speed,
particularly via ethtool paths, so this has always been unreliable (as
detailed in phylink's documentation.)

There is the additional issue that mtk_gmac0_rgmii_adjust() will only
be called if state->interface changes, which means it only configures
the system clocking on the very first .mac_config call, which will be
made when the network device is first brought up before any link is
established.

Essentially, this code is incredibly buggy, and probably never worked.

Moreover, checking the in-kernel DT files, it seems no platform makes
use of this code path.

Therefore, let's remove it, and disable interface modes for port 0 that
are not SGMII, 1000base-X, 2500base-X or TRGMII on the MT7623.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 59 ++++++---------------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  1 +
 2 files changed, 17 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 834c644b67db..46b80ec21b58 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -385,10 +385,8 @@ static int mt7621_gmac0_rgmii_adjust(struct mtk_eth *eth,
 }
 
 static void mtk_gmac0_rgmii_adjust(struct mtk_eth *eth,
-				   phy_interface_t interface, int speed)
+				   phy_interface_t interface)
 {
-	unsigned long rate;
-	u32 tck, rck, intf;
 	int ret;
 
 	if (interface == PHY_INTERFACE_MODE_TRGMII) {
@@ -399,30 +397,7 @@ static void mtk_gmac0_rgmii_adjust(struct mtk_eth *eth,
 		return;
 	}
 
-	if (speed == SPEED_1000) {
-		intf = INTF_MODE_RGMII_1000;
-		rate = 250000000;
-		rck = RCK_CTRL_RGMII_1000;
-		tck = TCK_CTRL_RGMII_1000;
-	} else {
-		intf = INTF_MODE_RGMII_10_100;
-		rate = 500000000;
-		rck = RCK_CTRL_RGMII_10_100;
-		tck = TCK_CTRL_RGMII_10_100;
-	}
-
-	mtk_w32(eth, intf, INTF_MODE);
-
-	regmap_update_bits(eth->ethsys, ETHSYS_CLKCFG0,
-			   ETHSYS_TRGMII_CLK_SEL362_5,
-			   ETHSYS_TRGMII_CLK_SEL362_5);
-
-	ret = clk_set_rate(eth->clks[MTK_CLK_TRGPLL], rate);
-	if (ret)
-		dev_err(eth->dev, "Failed to set trgmii pll: %d\n", ret);
-
-	mtk_w32(eth, rck, TRGMII_RCK_CTRL);
-	mtk_w32(eth, tck, TRGMII_TCK_CTRL);
+	dev_err(eth->dev, "Missing PLL configuration, ethernet may not work\n");
 }
 
 static struct phylink_pcs *mtk_mac_select_pcs(struct phylink_config *config,
@@ -498,17 +473,8 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode,
 							      state->interface))
 					goto err_phy;
 			} else {
-				/* FIXME: this is incorrect. Not only does it
-				 * use state->speed (which is not guaranteed
-				 * to be correct) but it also makes use of it
-				 * in a code path that will only be reachable
-				 * when the PHY interface mode changes, not
-				 * when the speed changes. Consequently, RGMII
-				 * is probably broken.
-				 */
 				mtk_gmac0_rgmii_adjust(mac->hw,
-						       state->interface,
-						       state->speed);
+						       state->interface);
 
 				/* mt7623_pad_clk_setup */
 				for (i = 0 ; i < NUM_TRGMII_CTRL; i++)
@@ -4373,13 +4339,19 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 	mac->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
 		MAC_10 | MAC_100 | MAC_1000 | MAC_2500FD;
 
-	__set_bit(PHY_INTERFACE_MODE_MII,
-		  mac->phylink_config.supported_interfaces);
-	__set_bit(PHY_INTERFACE_MODE_GMII,
-		  mac->phylink_config.supported_interfaces);
+	/* MT7623 gmac0 is now missing its speed-specific PLL configuration
+	 * in its .mac_config method (since state->speed is not valid there.
+	 * Disable support for MII, GMII and RGMII.
+	 */
+	if (!mac->hw->soc->disable_pll_modes || mac->id != 0) {
+		__set_bit(PHY_INTERFACE_MODE_MII,
+			  mac->phylink_config.supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_GMII,
+			  mac->phylink_config.supported_interfaces);
 
-	if (MTK_HAS_CAPS(mac->hw->soc->caps, MTK_RGMII))
-		phy_interface_set_rgmii(mac->phylink_config.supported_interfaces);
+		if (MTK_HAS_CAPS(mac->hw->soc->caps, MTK_RGMII))
+			phy_interface_set_rgmii(mac->phylink_config.supported_interfaces);
+	}
 
 	if (MTK_HAS_CAPS(mac->hw->soc->caps, MTK_TRGMII) && !mac->id)
 		__set_bit(PHY_INTERFACE_MODE_TRGMII,
@@ -4852,6 +4824,7 @@ static const struct mtk_soc_data mt7623_data = {
 	.offload_version = 1,
 	.hash_offset = 2,
 	.foe_entry_size = sizeof(struct mtk_foe_entry) - 16,
+	.disable_pll_modes = true,
 	.txrx = {
 		.txd_size = sizeof(struct mtk_tx_dma),
 		.rxd_size = sizeof(struct mtk_rx_dma),
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 707445f6bcb1..28adda0c90c0 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -1030,6 +1030,7 @@ struct mtk_soc_data {
 	u16		foe_entry_size;
 	netdev_features_t hw_features;
 	bool		has_accounting;
+	bool		disable_pll_modes;
 	struct {
 		u32	txd_size;
 		u32	rxd_size;
-- 
2.30.2

[PATCH net-next 2/4] net: ethernet: mtk_eth_soc: remove mac_pcs_get_state and modernise

[Russell King (Oracle)]

Remove the .mac_pcs_get_state function, since as far as I can tell is
never called - no DT appears to specify an in-band-status management
nor SFP support for this driver.

Removal of this, along with the previous patch to remove the incorrect
clocking configuration, means that the driver becomes non-legacy, so
we can remove the "legacy_pre_march2020" status from this driver.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 35 ---------------------
 1 file changed, 35 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 46b80ec21b58..0d5f47e2279e 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -568,38 +568,6 @@ static int mtk_mac_finish(struct phylink_config *config, unsigned int mode,
 	return 0;
 }
 
-static void mtk_mac_pcs_get_state(struct phylink_config *config,
-				  struct phylink_link_state *state)
-{
-	struct mtk_mac *mac = container_of(config, struct mtk_mac,
-					   phylink_config);
-	u32 pmsr = mtk_r32(mac->hw, MTK_MAC_MSR(mac->id));
-
-	state->link = (pmsr & MAC_MSR_LINK);
-	state->duplex = (pmsr & MAC_MSR_DPX) >> 1;
-
-	switch (pmsr & (MAC_MSR_SPEED_1000 | MAC_MSR_SPEED_100)) {
-	case 0:
-		state->speed = SPEED_10;
-		break;
-	case MAC_MSR_SPEED_100:
-		state->speed = SPEED_100;
-		break;
-	case MAC_MSR_SPEED_1000:
-		state->speed = SPEED_1000;
-		break;
-	default:
-		state->speed = SPEED_UNKNOWN;
-		break;
-	}
-
-	state->pause &= (MLO_PAUSE_RX | MLO_PAUSE_TX);
-	if (pmsr & MAC_MSR_RX_FC)
-		state->pause |= MLO_PAUSE_RX;
-	if (pmsr & MAC_MSR_TX_FC)
-		state->pause |= MLO_PAUSE_TX;
-}
-
 static void mtk_mac_link_down(struct phylink_config *config, unsigned int mode,
 			      phy_interface_t interface)
 {
@@ -722,7 +690,6 @@ static void mtk_mac_link_up(struct phylink_config *config,
 
 static const struct phylink_mac_ops mtk_phylink_ops = {
 	.mac_select_pcs = mtk_mac_select_pcs,
-	.mac_pcs_get_state = mtk_mac_pcs_get_state,
 	.mac_config = mtk_mac_config,
 	.mac_finish = mtk_mac_finish,
 	.mac_link_down = mtk_mac_link_down,
@@ -4334,8 +4301,6 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 
 	mac->phylink_config.dev = &eth->netdev[id]->dev;
 	mac->phylink_config.type = PHYLINK_NETDEV;
-	/* This driver makes use of state->speed in mac_config */
-	mac->phylink_config.legacy_pre_march2020 = true;
 	mac->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
 		MAC_10 | MAC_100 | MAC_1000 | MAC_2500FD;
 
-- 
2.30.2

[PATCH net-next 3/4] net: phylink: strip out pre-March 2020 legacy code

[Russell King (Oracle)]

Strip out all the pre-March 2020 legacy code from phylink now that the
last user of it is gone.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 25 ++++------------------
 include/linux/phylink.h   | 45 ++++++---------------------------------
 2 files changed, 10 insertions(+), 60 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index f07e496319b4..df413fb15088 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1198,13 +1198,6 @@ static int phylink_change_inband_advert(struct phylink *pl)
 	if (test_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state))
 		return 0;
 
-	if (!pl->pcs && pl->config->legacy_pre_march2020) {
-		/* Legacy method */
-		phylink_mac_config(pl, &pl->link_config);
-		phylink_pcs_an_restart(pl);
-		return 0;
-	}
-
 	phylink_dbg(pl, "%s: mode=%s/%s adv=%*pb pause=%02x\n", __func__,
 		    phylink_an_mode_str(pl->cur_link_an_mode),
 		    phy_modes(pl->link_config.interface),
@@ -1257,9 +1250,6 @@ static void phylink_mac_pcs_get_state(struct phylink *pl,
 
 	if (pl->pcs)
 		pl->pcs->ops->pcs_get_state(pl->pcs, state);
-	else if (pl->mac_ops->mac_pcs_get_state &&
-		 pl->config->legacy_pre_march2020)
-		pl->mac_ops->mac_pcs_get_state(pl->config, state);
 	else
 		state->link = 0;
 }
@@ -1492,13 +1482,6 @@ static void phylink_resolve(struct work_struct *w)
 			}
 			phylink_major_config(pl, false, &link_state);
 			pl->link_config.interface = link_state.interface;
-		} else if (!pl->pcs && pl->config->legacy_pre_march2020) {
-			/* The interface remains unchanged, only the speed,
-			 * duplex or pause settings have changed. Call the
-			 * old mac_config() method to configure the MAC/PCS
-			 * only if we do not have a legacy MAC driver.
-			 */
-			phylink_mac_config(pl, &link_state);
 		}
 	}
 
@@ -3513,7 +3496,7 @@ static void phylink_decode_usgmii_word(struct phylink_link_state *state,
  *
  * Parse the Clause 37 or Cisco SGMII link partner negotiation word into
  * the phylink @state structure. This is suitable to be used for implementing
- * the mac_pcs_get_state() member of the struct phylink_mac_ops structure if
+ * the pcs_get_state() member of the struct phylink_pcs_ops structure if
  * accessing @bmsr and @lpa cannot be done with MDIO directly.
  */
 void phylink_mii_c22_pcs_decode_state(struct phylink_link_state *state,
@@ -3563,7 +3546,7 @@ EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_decode_state);
  * Read the MAC PCS state from the MII device configured in @config and
  * parse the Clause 37 or Cisco SGMII link partner negotiation word into
  * the phylink @state structure. This is suitable to be directly plugged
- * into the mac_pcs_get_state() member of the struct phylink_mac_ops
+ * into the pcs_get_state() member of the struct phylink_pcs_ops
  * structure.
  */
 void phylink_mii_c22_pcs_get_state(struct mdio_device *pcs,
@@ -3674,8 +3657,8 @@ EXPORT_SYMBOL_GPL(phylink_mii_c22_pcs_config);
  * clause 37 negotiation.
  *
  * Restart the clause 37 negotiation with the link partner. This is
- * suitable to be directly plugged into the mac_pcs_get_state() member
- * of the struct phylink_mac_ops structure.
+ * suitable to be directly plugged into the pcs_get_state() member
+ * of the struct phylink_pcs_ops structure.
  */
 void phylink_mii_c22_pcs_an_restart(struct mdio_device *pcs)
 {
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 9e861c8316d0..789c516c6b4a 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -201,8 +201,6 @@ enum phylink_op_type {
  * struct phylink_config - PHYLINK configuration structure
  * @dev: a pointer to a struct device associated with the MAC
  * @type: operation type of PHYLINK instance
- * @legacy_pre_march2020: driver has not been updated for March 2020 updates
- *	(See commit 7cceb599d15d ("net: phylink: avoid mac_config calls")
  * @poll_fixed_state: if true, starts link_poll,
  *		      if MAC link is at %MLO_AN_FIXED mode.
  * @mac_managed_pm: if true, indicate the MAC driver is responsible for PHY PM.
@@ -216,7 +214,6 @@ enum phylink_op_type {
 struct phylink_config {
 	struct device *dev;
 	enum phylink_op_type type;
-	bool legacy_pre_march2020;
 	bool poll_fixed_state;
 	bool mac_managed_pm;
 	bool ovr_an_inband;
@@ -230,7 +227,6 @@ struct phylink_config {
  * struct phylink_mac_ops - MAC operations structure.
  * @validate: Validate and update the link configuration.
  * @mac_select_pcs: Select a PCS for the interface mode.
- * @mac_pcs_get_state: Read the current link state from the hardware.
  * @mac_prepare: prepare for a major reconfiguration of the interface.
  * @mac_config: configure the MAC for the selected mode and state.
  * @mac_finish: finish a major reconfiguration of the interface.
@@ -245,8 +241,6 @@ struct phylink_mac_ops {
 			 struct phylink_link_state *state);
 	struct phylink_pcs *(*mac_select_pcs)(struct phylink_config *config,
 					      phy_interface_t interface);
-	void (*mac_pcs_get_state)(struct phylink_config *config,
-				  struct phylink_link_state *state);
 	int (*mac_prepare)(struct phylink_config *config, unsigned int mode,
 			   phy_interface_t iface);
 	void (*mac_config)(struct phylink_config *config, unsigned int mode,
@@ -312,25 +306,6 @@ void validate(struct phylink_config *config, unsigned long *supported,
 struct phylink_pcs *mac_select_pcs(struct phylink_config *config,
 				   phy_interface_t interface);
 
-/**
- * mac_pcs_get_state() - Read the current inband link state from the hardware
- * @config: a pointer to a &struct phylink_config.
- * @state: a pointer to a &struct phylink_link_state.
- *
- * Read the current inband link state from the MAC PCS, reporting the
- * current speed in @state->speed, duplex mode in @state->duplex, pause
- * mode in @state->pause using the %MLO_PAUSE_RX and %MLO_PAUSE_TX bits,
- * negotiation completion state in @state->an_complete, and link up state
- * in @state->link. If possible, @state->lp_advertising should also be
- * populated.
- *
- * Note: This is a legacy method. This function will not be called unless
- * legacy_pre_march2020 is set in &struct phylink_config and there is no
- * PCS attached.
- */
-void mac_pcs_get_state(struct phylink_config *config,
-		       struct phylink_link_state *state);
-
 /**
  * mac_prepare() - prepare to change the PHY interface mode
  * @config: a pointer to a &struct phylink_config.
@@ -367,17 +342,9 @@ int mac_prepare(struct phylink_config *config, unsigned int mode,
  * guaranteed to be correct, and so any mac_config() implementation must
  * never reference these fields.
  *
- * Note: For legacy March 2020 drivers (drivers with legacy_pre_march2020 set
- * in their &phylnk_config and which don't have a PCS), this function will be
- * called on each link up event, and to also change the in-band advert. For
- * non-legacy drivers, it will only be called to reconfigure the MAC for a
- * "major" change in e.g. interface mode. It will not be called for changes
- * in speed, duplex or pause modes or to change the in-band advertisement.
- * In any case, it is strongly preferred that speed, duplex and pause settings
- * are handled in the mac_link_up() method and not in this method.
- *
- * (this requires a rewrite - please refer to mac_link_up() for situations
- *  where the PCS and MAC are not tightly integrated.)
+ * This will only be called to reconfigure the MAC for a "major" change in
+ * e.g. interface mode. It will not be called for changes in speed, duplex
+ * or pause modes or to change the in-band advertisement.
  *
  * In all negotiation modes, as defined by @mode, @state->pause indicates the
  * pause settings which should be applied as follows. If %MLO_PAUSE_AN is not
@@ -409,7 +376,7 @@ int mac_prepare(struct phylink_config *config, unsigned int mode,
  *   1000base-X or Cisco SGMII mode depending on the @state->interface
  *   mode). In both cases, link state management (whether the link
  *   is up or not) is performed by the MAC, and reported via the
- *   mac_pcs_get_state() callback. Changes in link state must be made
+ *   pcs_get_state() callback. Changes in link state must be made
  *   by calling phylink_mac_change().
  *
  *   Interface mode specific details are mentioned below.
@@ -601,8 +568,8 @@ void pcs_disable(struct phylink_pcs *pcs);
  * in @state->link. If possible, @state->lp_advertising should also be
  * populated.
  *
- * When present, this overrides mac_pcs_get_state() in &struct
- * phylink_mac_ops.
+ * When present, this overrides pcs_get_state() in &struct
+ * phylink_pcs_ops.
  */
 void pcs_get_state(struct phylink_pcs *pcs,
 		   struct phylink_link_state *state);
-- 
2.30.2

[PATCH net-next 4/4] net: phylink: explicitly invalidate link_state members in mac_config

[Russell King (Oracle)]

Explicitly invalidate the phylink_link_state structure members in
mac_config that do not contain reliable information for this function,
thereby preventing their future incorrect use.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index df413fb15088..4f1c8bb199e9 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1066,17 +1066,24 @@ static void phylink_pcs_poll_start(struct phylink *pl)
 static void phylink_mac_config(struct phylink *pl,
 			       const struct phylink_link_state *state)
 {
+	struct phylink_link_state st = *state;
+
+	/* Stop drivers incorrectly using these */
+	linkmode_zero(st.lp_advertising);
+	st.speed = SPEED_UNKNOWN;
+	st.duplex = DUPLEX_UNKNOWN;
+	st.an_complete = false;
+	st.link = false;
+
 	phylink_dbg(pl,
-		    "%s: mode=%s/%s/%s/%s/%s adv=%*pb pause=%02x link=%u\n",
+		    "%s: mode=%s/%s/%s adv=%*pb pause=%02x\n",
 		    __func__, phylink_an_mode_str(pl->cur_link_an_mode),
-		    phy_modes(state->interface),
-		    phy_speed_to_str(state->speed),
-		    phy_duplex_to_str(state->duplex),
-		    phy_rate_matching_to_str(state->rate_matching),
-		    __ETHTOOL_LINK_MODE_MASK_NBITS, state->advertising,
-		    state->pause, state->link);
-
-	pl->mac_ops->mac_config(pl->config, pl->cur_link_an_mode, state);
+		    phy_modes(st.interface),
+		    phy_rate_matching_to_str(st.rate_matching),
+		    __ETHTOOL_LINK_MODE_MASK_NBITS, st.advertising,
+		    st.pause);
+
+	pl->mac_ops->mac_config(pl->config, pl->cur_link_an_mode, &st);
 }
 
 static void phylink_pcs_an_restart(struct phylink *pl)
-- 
2.30.2

Re: [PATCH net-next 0/4] Remove legacy phylink behaviour

[Russell King (Oracle)]

On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote:
> Hi,
> 
> This series removes the - as far as I can tell - unreachable code in
> mtk_eth_soc that relies upon legacy phylink behaviour, and then removes
> the support in phylink for this legacy behaviour.
> 
> Patch 1 removes the clocking configuration from mtk_eth_soc for non-
> TRGMII, non-serdes based interface modes, and disables those interface
> modes prior to phylink configuration.
> 
> Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which
> I believe is also not used - mtk_eth_soc appears not to be used with
> SFPs (which would use a kind of in-band mode) nor does any DT appear
> to specify in-band mode for any non-serdes based interface mode.
> 
> With both of those dealt with, the kernel is now free of any driver
> relying on the phylink legacy mode. Therefore, patch 3 removes support
> for this.
> 
> Finally, with the advent of a new driver being submitted today that
> makes use of state->speed in the mac_config() path, patch 4 ensures that
> any phylink_link_state member that should not be used in mac_config is
> either cleared or set to an invalid value.

Note: probably be a good idea for the mtk_eth_soc people to review and
ack before applying this to net-next.

Thanks.

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

Re: [PATCH net-next 0/4] Remove legacy phylink behaviour

[Daniel Golle]

Hi Russell,

On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote:
> Hi,
> 
> This series removes the - as far as I can tell - unreachable code in
> mtk_eth_soc that relies upon legacy phylink behaviour, and then removes
> the support in phylink for this legacy behaviour.
> 
> Patch 1 removes the clocking configuration from mtk_eth_soc for non-
> TRGMII, non-serdes based interface modes, and disables those interface
> modes prior to phylink configuration.
> 
> Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which
> I believe is also not used - mtk_eth_soc appears not to be used with
> SFPs (which would use a kind of in-band mode) nor does any DT appear
> to specify in-band mode for any non-serdes based interface mode.
> 
> With both of those dealt with, the kernel is now free of any driver
> relying on the phylink legacy mode. Therefore, patch 3 removes support
> for this.
> 
> Finally, with the advent of a new driver being submitted today that
> makes use of state->speed in the mac_config() path, patch 4 ensures that
> any phylink_link_state member that should not be used in mac_config is
> either cleared or set to an invalid value.

Thank you for taking care of this!

For the whole series:

Reviewed-by: Daniel Golle <daniel@makrotopia.org>
Tested-by: Daniel Golle <daniel@makrotopia.org>

Tested on BPi-R2 (MT7623N), BPi-R3 (MT7986A) and BPi-R64 (MT7622A).
All works fine as expected.

To apply the series I needed to resolve a minor conflict due to
net: ethernet: mtk_ppe: add MTK_FOE_ENTRY_V{1,2}_SIZE macros
being applied in the meantime.


> 
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c | 94 +++++------------------------
>  drivers/net/ethernet/mediatek/mtk_eth_soc.h |  1 +
>  drivers/net/phy/phylink.c                   | 48 ++++++---------
>  include/linux/phylink.h                     | 45 ++------------
>  4 files changed, 42 insertions(+), 146 deletions(-)
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Aw: Re: [PATCH net-next 0/4] Remove legacy phylink behaviour

[Frank Wunderlich]

Hi

> Gesendet: Samstag, 22. Juli 2023 um 00:40 Uhr
> Von: "Daniel Golle" <daniel@makrotopia.org>
>
> Hi Russell,
>
> On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote:
> > Hi,
> >
> > This series removes the - as far as I can tell - unreachable code in
> > mtk_eth_soc that relies upon legacy phylink behaviour, and then removes
> > the support in phylink for this legacy behaviour.
> >
> > Patch 1 removes the clocking configuration from mtk_eth_soc for non-
> > TRGMII, non-serdes based interface modes, and disables those interface
> > modes prior to phylink configuration.
> >
> > Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which
> > I believe is also not used - mtk_eth_soc appears not to be used with
> > SFPs (which would use a kind of in-band mode) nor does any DT appear
> > to specify in-band mode for any non-serdes based interface mode.
> >
> > With both of those dealt with, the kernel is now free of any driver
> > relying on the phylink legacy mode. Therefore, patch 3 removes support
> > for this.
> >
> > Finally, with the advent of a new driver being submitted today that
> > makes use of state->speed in the mac_config() path, patch 4 ensures that
> > any phylink_link_state member that should not be used in mac_config is
> > either cleared or set to an invalid value.
>
> Thank you for taking care of this!

> For the whole series:
>
> Reviewed-by: Daniel Golle <daniel@makrotopia.org>
> Tested-by: Daniel Golle <daniel@makrotopia.org>
>
> Tested on BPi-R2 (MT7623N), BPi-R3 (MT7986A) and BPi-R64 (MT7622A).
> All works fine as expected.

have you changed anything?

in my test with bpi-r2 i see boot hangs after link-up on my wan-port (still trgmii-mode configured), no access to userspace.

[   10.881844] mtk_soc_eth 1b100000.ethernet eth0: configuring for fixed/trgmii link mode
[   10.891611] mtk_soc_eth 1b100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
[   11.005814] mt7530-mdio mdio-bus:1f wan: configuring for phy/gmii link mode
[   11.016654] mt7530-mdio mdio-bus:1f lan3: configuring for phy/gmii link mode
[   11.025685] mt7530-mdio mdio-bus:1f lan2: configuring for phy/gmii link mode
[   11.035122] mt7530-mdio mdio-bus:1f lan1: configuring for phy/gmii link mode
[   11.045370] mt7530-mdio mdio-bus:1f lan0: configuring for phy/gmii link mode

[   15.144255] mt7530-mdio mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx

removing the cable does not show link down so it looks like completely stalled...but also no panic/crash or similar.
When booting without cable it hangs too (without the up-message of course).

> To apply the series I needed to resolve a minor conflict due to
> net: ethernet: mtk_ppe: add MTK_FOE_ENTRY_V{1,2}_SIZE macros
> being applied in the meantime.

i used yesterdays net-next (6.5-rc2) and put series on top (also needed to do one hunk of first patch manually) and
then my defconfig,buildscript etc. maybe i miss anything?

afair trgmii is basicly rgmii only with higher clock-setting...and if this is dropped from mac-driver but switch is still using it it cannot work, but i'm sure you know this ;)

regards Frank

Re: Aw: Re: [PATCH net-next 0/4] Remove legacy phylink behaviour

[Daniel Golle]

On Sat, Jul 22, 2023 at 02:31:45PM +0200, Frank Wunderlich wrote:
> Hi
> 
> > Gesendet: Samstag, 22. Juli 2023 um 00:40 Uhr
> > Von: "Daniel Golle" <daniel@makrotopia.org>
> >
> > Hi Russell,
> >
> > On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote:
> > > Hi,
> > >
> > > This series removes the - as far as I can tell - unreachable code in
> > > mtk_eth_soc that relies upon legacy phylink behaviour, and then removes
> > > the support in phylink for this legacy behaviour.
> > >
> > > Patch 1 removes the clocking configuration from mtk_eth_soc for non-
> > > TRGMII, non-serdes based interface modes, and disables those interface
> > > modes prior to phylink configuration.
> > >
> > > Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which
> > > I believe is also not used - mtk_eth_soc appears not to be used with
> > > SFPs (which would use a kind of in-band mode) nor does any DT appear
> > > to specify in-band mode for any non-serdes based interface mode.
> > >
> > > With both of those dealt with, the kernel is now free of any driver
> > > relying on the phylink legacy mode. Therefore, patch 3 removes support
> > > for this.
> > >
> > > Finally, with the advent of a new driver being submitted today that
> > > makes use of state->speed in the mac_config() path, patch 4 ensures that
> > > any phylink_link_state member that should not be used in mac_config is
> > > either cleared or set to an invalid value.
> >
> > Thank you for taking care of this!
> 
> > For the whole series:
> >
> > Reviewed-by: Daniel Golle <daniel@makrotopia.org>
> > Tested-by: Daniel Golle <daniel@makrotopia.org>
> >
> > Tested on BPi-R2 (MT7623N), BPi-R3 (MT7986A) and BPi-R64 (MT7622A).
> > All works fine as expected.
> 
> have you changed anything?
> 
> in my test with bpi-r2 i see boot hangs after link-up on my wan-port (still trgmii-mode configured), no access to userspace.
> 
> [   10.881844] mtk_soc_eth 1b100000.ethernet eth0: configuring for fixed/trgmii link mode
> [   10.891611] mtk_soc_eth 1b100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
> [   11.005814] mt7530-mdio mdio-bus:1f wan: configuring for phy/gmii link mode
> [   11.016654] mt7530-mdio mdio-bus:1f lan3: configuring for phy/gmii link mode
> [   11.025685] mt7530-mdio mdio-bus:1f lan2: configuring for phy/gmii link mode
> [   11.035122] mt7530-mdio mdio-bus:1f lan1: configuring for phy/gmii link mode
> [   11.045370] mt7530-mdio mdio-bus:1f lan0: configuring for phy/gmii link mode
> 
> [   15.144255] mt7530-mdio mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx
> 
> removing the cable does not show link down so it looks like completely stalled...but also no panic/crash or similar.
> When booting without cable it hangs too (without the up-message of course).
> 
> > To apply the series I needed to resolve a minor conflict due to
> > net: ethernet: mtk_ppe: add MTK_FOE_ENTRY_V{1,2}_SIZE macros
> > being applied in the meantime.
> 
> i used yesterdays net-next (6.5-rc2) and put series on top (also needed to do one hunk of first patch manually) and
> then my defconfig,buildscript etc. maybe i miss anything?

I tested in a fairly similar way.

> 
> afair trgmii is basicly rgmii only with higher clock-setting...and if this is dropped from mac-driver but switch is still using it it cannot work, but i'm sure you know this ;)

For TRGMII mode mtk_gmac0_rgmii_adjust behavior should not have changed,
only speed-dependent adjustments for RGMII have been dropped.
I also didn't not see such a problem when I was testing.

However, I guess in my case the MAC interface mode has already been
setup in hardware because I booted the board via TFTP, hence U-Boot
already took care of initializing everything, so that can also be
a reason why it worked fine for me.

Re: Re: [PATCH net-next 0/4] Remove legacy phylink behaviour

[Russell King (Oracle)]

On Sat, Jul 22, 2023 at 02:31:45PM +0200, Frank Wunderlich wrote:
> Hi
> 
> > Gesendet: Samstag, 22. Juli 2023 um 00:40 Uhr
> > Von: "Daniel Golle" <daniel@makrotopia.org>
> >
> > Hi Russell,
> >
> > On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote:
> > > Hi,
> > >
> > > This series removes the - as far as I can tell - unreachable code in
> > > mtk_eth_soc that relies upon legacy phylink behaviour, and then removes
> > > the support in phylink for this legacy behaviour.
> > >
> > > Patch 1 removes the clocking configuration from mtk_eth_soc for non-
> > > TRGMII, non-serdes based interface modes, and disables those interface
> > > modes prior to phylink configuration.
> > >
> > > Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which
> > > I believe is also not used - mtk_eth_soc appears not to be used with
> > > SFPs (which would use a kind of in-band mode) nor does any DT appear
> > > to specify in-band mode for any non-serdes based interface mode.
> > >
> > > With both of those dealt with, the kernel is now free of any driver
> > > relying on the phylink legacy mode. Therefore, patch 3 removes support
> > > for this.
> > >
> > > Finally, with the advent of a new driver being submitted today that
> > > makes use of state->speed in the mac_config() path, patch 4 ensures that
> > > any phylink_link_state member that should not be used in mac_config is
> > > either cleared or set to an invalid value.
> >
> > Thank you for taking care of this!
> 
> > For the whole series:
> >
> > Reviewed-by: Daniel Golle <daniel@makrotopia.org>
> > Tested-by: Daniel Golle <daniel@makrotopia.org>
> >
> > Tested on BPi-R2 (MT7623N), BPi-R3 (MT7986A) and BPi-R64 (MT7622A).
> > All works fine as expected.
> 
> have you changed anything?
> 
> in my test with bpi-r2 i see boot hangs after link-up on my wan-port (still trgmii-mode configured), no access to userspace.
> 
> [   10.881844] mtk_soc_eth 1b100000.ethernet eth0: configuring for fixed/trgmii link mode
> [   10.891611] mtk_soc_eth 1b100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
> [   11.005814] mt7530-mdio mdio-bus:1f wan: configuring for phy/gmii link mode
> [   11.016654] mt7530-mdio mdio-bus:1f lan3: configuring for phy/gmii link mode
> [   11.025685] mt7530-mdio mdio-bus:1f lan2: configuring for phy/gmii link mode
> [   11.035122] mt7530-mdio mdio-bus:1f lan1: configuring for phy/gmii link mode
> [   11.045370] mt7530-mdio mdio-bus:1f lan0: configuring for phy/gmii link mode
> 
> [   15.144255] mt7530-mdio mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx
> 
> removing the cable does not show link down so it looks like completely stalled...but also no panic/crash or similar.
> When booting without cable it hangs too (without the up-message of course).
> 
> > To apply the series I needed to resolve a minor conflict due to
> > net: ethernet: mtk_ppe: add MTK_FOE_ENTRY_V{1,2}_SIZE macros
> > being applied in the meantime.
> 
> i used yesterdays net-next (6.5-rc2) and put series on top (also needed to do one hunk of first patch manually) and
> then my defconfig,buildscript etc. maybe i miss anything?
> 
> afair trgmii is basicly rgmii only with higher clock-setting...and if this is dropped from mac-driver but switch is still using it it cannot work, but i'm sure you know this ;)

Have you tried bisecting the four patches? Does that give any hints?
Does the net-next base you used boot successfully?

Thanks.

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

Aw: Re: Re: [PATCH net-next 0/4] Remove legacy phylink behaviour

[Frank Wunderlich]

> Gesendet: Samstag, 22. Juli 2023 um 18:14 Uhr
> Von: "Russell King (Oracle)" <linux@armlinux.org.uk>
>
> On Sat, Jul 22, 2023 at 02:31:45PM +0200, Frank Wunderlich wrote:
> > Hi
> >
> > > Gesendet: Samstag, 22. Juli 2023 um 00:40 Uhr
> > > Von: "Daniel Golle" <daniel@makrotopia.org>
> > >
> > > Hi Russell,
> > >
> > > On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote:
> > > > Hi,
> > > >
> > > > This series removes the - as far as I can tell - unreachable code in
> > > > mtk_eth_soc that relies upon legacy phylink behaviour, and then removes
> > > > the support in phylink for this legacy behaviour.
> > > >
> > > > Patch 1 removes the clocking configuration from mtk_eth_soc for non-
> > > > TRGMII, non-serdes based interface modes, and disables those interface
> > > > modes prior to phylink configuration.
> > > >
> > > > Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which
> > > > I believe is also not used - mtk_eth_soc appears not to be used with
> > > > SFPs (which would use a kind of in-band mode) nor does any DT appear
> > > > to specify in-band mode for any non-serdes based interface mode.
> > > >
> > > > With both of those dealt with, the kernel is now free of any driver
> > > > relying on the phylink legacy mode. Therefore, patch 3 removes support
> > > > for this.
> > > >
> > > > Finally, with the advent of a new driver being submitted today that
> > > > makes use of state->speed in the mac_config() path, patch 4 ensures that
> > > > any phylink_link_state member that should not be used in mac_config is
> > > > either cleared or set to an invalid value.
> > >
> > > Thank you for taking care of this!
> >
> > > For the whole series:
> > >
> > > Reviewed-by: Daniel Golle <daniel@makrotopia.org>
> > > Tested-by: Daniel Golle <daniel@makrotopia.org>
> > >
> > > Tested on BPi-R2 (MT7623N), BPi-R3 (MT7986A) and BPi-R64 (MT7622A).
> > > All works fine as expected.
> >
> > have you changed anything?
> >
> > in my test with bpi-r2 i see boot hangs after link-up on my wan-port (still trgmii-mode configured), no access to userspace.
> >
> > [   10.881844] mtk_soc_eth 1b100000.ethernet eth0: configuring for fixed/trgmii link mode
> > [   10.891611] mtk_soc_eth 1b100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
> > [   11.005814] mt7530-mdio mdio-bus:1f wan: configuring for phy/gmii link mode
> > [   11.016654] mt7530-mdio mdio-bus:1f lan3: configuring for phy/gmii link mode
> > [   11.025685] mt7530-mdio mdio-bus:1f lan2: configuring for phy/gmii link mode
> > [   11.035122] mt7530-mdio mdio-bus:1f lan1: configuring for phy/gmii link mode
> > [   11.045370] mt7530-mdio mdio-bus:1f lan0: configuring for phy/gmii link mode
> >
> > [   15.144255] mt7530-mdio mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx
> >
> > removing the cable does not show link down so it looks like completely stalled...but also no panic/crash or similar.
> > When booting without cable it hangs too (without the up-message of course).
> >
> > > To apply the series I needed to resolve a minor conflict due to
> > > net: ethernet: mtk_ppe: add MTK_FOE_ENTRY_V{1,2}_SIZE macros
> > > being applied in the meantime.
> >
> > i used yesterdays net-next (6.5-rc2) and put series on top (also needed to do one hunk of first patch manually) and
> > then my defconfig,buildscript etc. maybe i miss anything?
> >
> > afair trgmii is basicly rgmii only with higher clock-setting...and if this is dropped from mac-driver but switch is still using it it cannot work, but i'm sure you know this ;)
>
> Have you tried bisecting the four patches? Does that give any hints?
> Does the net-next base you used boot successfully?

sorry, hang was caused by me (missing rootfs and wait in bootargs).

only see throughput of only ~850Mbit (tx) and 900Mbit (rx)..wan is on eth0 which is the trgmii one, but without the patches it is similar (quicktested 6.3-rc1 i had already on this card which has similar results)

so for me it looks good so far...i'm not sure we had more than 900 anytime on this device...

regards Frank

Re: Re: Re: [PATCH net-next 0/4] Remove legacy phylink behaviour

[Russell King (Oracle)]

On Sat, Jul 22, 2023 at 08:00:46PM +0200, Frank Wunderlich wrote:
> 
> > Gesendet: Samstag, 22. Juli 2023 um 18:14 Uhr
> > Von: "Russell King (Oracle)" <linux@armlinux.org.uk>
> >
> > On Sat, Jul 22, 2023 at 02:31:45PM +0200, Frank Wunderlich wrote:
> > > Hi
> > >
> > > > Gesendet: Samstag, 22. Juli 2023 um 00:40 Uhr
> > > > Von: "Daniel Golle" <daniel@makrotopia.org>
> > > >
> > > > Hi Russell,
> > > >
> > > > On Fri, Jul 21, 2023 at 12:33:52PM +0100, Russell King (Oracle) wrote:
> > > > > Hi,
> > > > >
> > > > > This series removes the - as far as I can tell - unreachable code in
> > > > > mtk_eth_soc that relies upon legacy phylink behaviour, and then removes
> > > > > the support in phylink for this legacy behaviour.
> > > > >
> > > > > Patch 1 removes the clocking configuration from mtk_eth_soc for non-
> > > > > TRGMII, non-serdes based interface modes, and disables those interface
> > > > > modes prior to phylink configuration.
> > > > >
> > > > > Patch 2 removes the mac_pcs_get_state() method from mtk_eth_soc which
> > > > > I believe is also not used - mtk_eth_soc appears not to be used with
> > > > > SFPs (which would use a kind of in-band mode) nor does any DT appear
> > > > > to specify in-band mode for any non-serdes based interface mode.
> > > > >
> > > > > With both of those dealt with, the kernel is now free of any driver
> > > > > relying on the phylink legacy mode. Therefore, patch 3 removes support
> > > > > for this.
> > > > >
> > > > > Finally, with the advent of a new driver being submitted today that
> > > > > makes use of state->speed in the mac_config() path, patch 4 ensures that
> > > > > any phylink_link_state member that should not be used in mac_config is
> > > > > either cleared or set to an invalid value.
> > > >
> > > > Thank you for taking care of this!
> > >
> > > > For the whole series:
> > > >
> > > > Reviewed-by: Daniel Golle <daniel@makrotopia.org>
> > > > Tested-by: Daniel Golle <daniel@makrotopia.org>
> > > >
> > > > Tested on BPi-R2 (MT7623N), BPi-R3 (MT7986A) and BPi-R64 (MT7622A).
> > > > All works fine as expected.
> > >
> > > have you changed anything?
> > >
> > > in my test with bpi-r2 i see boot hangs after link-up on my wan-port (still trgmii-mode configured), no access to userspace.
> > >
> > > [   10.881844] mtk_soc_eth 1b100000.ethernet eth0: configuring for fixed/trgmii link mode
> > > [   10.891611] mtk_soc_eth 1b100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
> > > [   11.005814] mt7530-mdio mdio-bus:1f wan: configuring for phy/gmii link mode
> > > [   11.016654] mt7530-mdio mdio-bus:1f lan3: configuring for phy/gmii link mode
> > > [   11.025685] mt7530-mdio mdio-bus:1f lan2: configuring for phy/gmii link mode
> > > [   11.035122] mt7530-mdio mdio-bus:1f lan1: configuring for phy/gmii link mode
> > > [   11.045370] mt7530-mdio mdio-bus:1f lan0: configuring for phy/gmii link mode
> > >
> > > [   15.144255] mt7530-mdio mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx
> > >
> > > removing the cable does not show link down so it looks like completely stalled...but also no panic/crash or similar.
> > > When booting without cable it hangs too (without the up-message of course).
> > >
> > > > To apply the series I needed to resolve a minor conflict due to
> > > > net: ethernet: mtk_ppe: add MTK_FOE_ENTRY_V{1,2}_SIZE macros
> > > > being applied in the meantime.
> > >
> > > i used yesterdays net-next (6.5-rc2) and put series on top (also needed to do one hunk of first patch manually) and
> > > then my defconfig,buildscript etc. maybe i miss anything?
> > >
> > > afair trgmii is basicly rgmii only with higher clock-setting...and if this is dropped from mac-driver but switch is still using it it cannot work, but i'm sure you know this ;)
> >
> > Have you tried bisecting the four patches? Does that give any hints?
> > Does the net-next base you used boot successfully?
> 
> sorry, hang was caused by me (missing rootfs and wait in bootargs).
> 
> only see throughput of only ~850Mbit (tx) and 900Mbit (rx)..wan is on eth0 which is the trgmii one, but without the patches it is similar (quicktested 6.3-rc1 i had already on this card which has similar results)
> 
> so for me it looks good so far...i'm not sure we had more than 900 anytime on this device...

So I take it that nothing broke as a result of the patches, and
as you've tested them, I can add your tested-by to the patches?

Thanks.

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

[PATCH net-next 1/4] net: ethernet: mtk_eth_soc: remove incorrect PLL configuration

[Russell King (Oracle)]

MT7623 GMAC0 attempts to configure the system clocking according to the
required speed in the .mac_config callback for non-SGMII, non-baseX and
non-TRGMII modes.

state->speed setting has never been reliable in the .mac_config
callback - there are cases where this is not the link speed,
particularly via ethtool paths, so this has always been unreliable (as
detailed in phylink's documentation.)

There is the additional issue that mtk_gmac0_rgmii_adjust() will only
be called if state->interface changes, which means it only configures
the system clocking on the very first .mac_config call, which will be
made when the network device is first brought up before any link is
established.

Essentially, this code is incredibly buggy, and probably never worked.

Moreover, checking the in-kernel DT files, it seems no platform makes
use of this code path.

Therefore, let's remove it, and disable interface modes for port 0 that
are not SGMII, 1000base-X, 2500base-X or TRGMII on the MT7623.

Reviewed-by: Daniel Golle <daniel@makrotopia.org>
Tested-by: Daniel Golle <daniel@makrotopia.org>
Tested-by: Frank Wunderlich <frank-w@public-files.de>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 59 ++++++---------------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  1 +
 2 files changed, 17 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 18a8aca7944d..fe8b7e38decc 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -385,10 +385,8 @@ static int mt7621_gmac0_rgmii_adjust(struct mtk_eth *eth,
 }
 
 static void mtk_gmac0_rgmii_adjust(struct mtk_eth *eth,
-				   phy_interface_t interface, int speed)
+				   phy_interface_t interface)
 {
-	unsigned long rate;
-	u32 tck, rck, intf;
 	int ret;
 
 	if (interface == PHY_INTERFACE_MODE_TRGMII) {
@@ -399,30 +397,7 @@ static void mtk_gmac0_rgmii_adjust(struct mtk_eth *eth,
 		return;
 	}
 
-	if (speed == SPEED_1000) {
-		intf = INTF_MODE_RGMII_1000;
-		rate = 250000000;
-		rck = RCK_CTRL_RGMII_1000;
-		tck = TCK_CTRL_RGMII_1000;
-	} else {
-		intf = INTF_MODE_RGMII_10_100;
-		rate = 500000000;
-		rck = RCK_CTRL_RGMII_10_100;
-		tck = TCK_CTRL_RGMII_10_100;
-	}
-
-	mtk_w32(eth, intf, INTF_MODE);
-
-	regmap_update_bits(eth->ethsys, ETHSYS_CLKCFG0,
-			   ETHSYS_TRGMII_CLK_SEL362_5,
-			   ETHSYS_TRGMII_CLK_SEL362_5);
-
-	ret = clk_set_rate(eth->clks[MTK_CLK_TRGPLL], rate);
-	if (ret)
-		dev_err(eth->dev, "Failed to set trgmii pll: %d\n", ret);
-
-	mtk_w32(eth, rck, TRGMII_RCK_CTRL);
-	mtk_w32(eth, tck, TRGMII_TCK_CTRL);
+	dev_err(eth->dev, "Missing PLL configuration, ethernet may not work\n");
 }
 
 static struct phylink_pcs *mtk_mac_select_pcs(struct phylink_config *config,
@@ -498,17 +473,8 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode,
 							      state->interface))
 					goto err_phy;
 			} else {
-				/* FIXME: this is incorrect. Not only does it
-				 * use state->speed (which is not guaranteed
-				 * to be correct) but it also makes use of it
-				 * in a code path that will only be reachable
-				 * when the PHY interface mode changes, not
-				 * when the speed changes. Consequently, RGMII
-				 * is probably broken.
-				 */
 				mtk_gmac0_rgmii_adjust(mac->hw,
-						       state->interface,
-						       state->speed);
+						       state->interface);
 
 				/* mt7623_pad_clk_setup */
 				for (i = 0 ; i < NUM_TRGMII_CTRL; i++)
@@ -4366,13 +4332,19 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 	mac->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
 		MAC_10 | MAC_100 | MAC_1000 | MAC_2500FD;
 
-	__set_bit(PHY_INTERFACE_MODE_MII,
-		  mac->phylink_config.supported_interfaces);
-	__set_bit(PHY_INTERFACE_MODE_GMII,
-		  mac->phylink_config.supported_interfaces);
+	/* MT7623 gmac0 is now missing its speed-specific PLL configuration
+	 * in its .mac_config method (since state->speed is not valid there.
+	 * Disable support for MII, GMII and RGMII.
+	 */
+	if (!mac->hw->soc->disable_pll_modes || mac->id != 0) {
+		__set_bit(PHY_INTERFACE_MODE_MII,
+			  mac->phylink_config.supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_GMII,
+			  mac->phylink_config.supported_interfaces);
 
-	if (MTK_HAS_CAPS(mac->hw->soc->caps, MTK_RGMII))
-		phy_interface_set_rgmii(mac->phylink_config.supported_interfaces);
+		if (MTK_HAS_CAPS(mac->hw->soc->caps, MTK_RGMII))
+			phy_interface_set_rgmii(mac->phylink_config.supported_interfaces);
+	}
 
 	if (MTK_HAS_CAPS(mac->hw->soc->caps, MTK_TRGMII) && !mac->id)
 		__set_bit(PHY_INTERFACE_MODE_TRGMII,
@@ -4845,6 +4817,7 @@ static const struct mtk_soc_data mt7623_data = {
 	.offload_version = 1,
 	.hash_offset = 2,
 	.foe_entry_size = MTK_FOE_ENTRY_V1_SIZE,
+	.disable_pll_modes = true,
 	.txrx = {
 		.txd_size = sizeof(struct mtk_tx_dma),
 		.rxd_size = sizeof(struct mtk_rx_dma),
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 707445f6bcb1..28adda0c90c0 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -1030,6 +1030,7 @@ struct mtk_soc_data {
 	u16		foe_entry_size;
 	netdev_features_t hw_features;
 	bool		has_accounting;
+	bool		disable_pll_modes;
 	struct {
 		u32	txd_size;
 		u32	rxd_size;
-- 
2.30.2

[PATCH net-next 2/4] net: ethernet: mtk_eth_soc: remove mac_pcs_get_state and modernise

[Russell King (Oracle)]

Remove the .mac_pcs_get_state function, since as far as I can tell is
never called - no DT appears to specify an in-band-status management
nor SFP support for this driver.

Removal of this, along with the previous patch to remove the incorrect
clocking configuration, means that the driver becomes non-legacy, so
we can remove the "legacy_pre_march2020" status from this driver.

Reviewed-by: Daniel Golle <daniel@makrotopia.org>
Tested-by: Daniel Golle <daniel@makrotopia.org>
Tested-by: Frank Wunderlich <frank-w@public-files.de>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 35 ---------------------
 1 file changed, 35 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index fe8b7e38decc..7490d48000c2 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -568,38 +568,6 @@ static int mtk_mac_finish(struct phylink_config *config, unsigned int mode,
 	return 0;
 }
 
-static void mtk_mac_pcs_get_state(struct phylink_config *config,
-				  struct phylink_link_state *state)
-{
-	struct mtk_mac *mac = container_of(config, struct mtk_mac,
-					   phylink_config);
-	u32 pmsr = mtk_r32(mac->hw, MTK_MAC_MSR(mac->id));
-
-	state->link = (pmsr & MAC_MSR_LINK);
-	state->duplex = (pmsr & MAC_MSR_DPX) >> 1;
-
-	switch (pmsr & (MAC_MSR_SPEED_1000 | MAC_MSR_SPEED_100)) {
-	case 0:
-		state->speed = SPEED_10;
-		break;
-	case MAC_MSR_SPEED_100:
-		state->speed = SPEED_100;
-		break;
-	case MAC_MSR_SPEED_1000:
-		state->speed = SPEED_1000;
-		break;
-	default:
-		state->speed = SPEED_UNKNOWN;
-		break;
-	}
-
-	state->pause &= (MLO_PAUSE_RX | MLO_PAUSE_TX);
-	if (pmsr & MAC_MSR_RX_FC)
-		state->pause |= MLO_PAUSE_RX;
-	if (pmsr & MAC_MSR_TX_FC)
-		state->pause |= MLO_PAUSE_TX;
-}
-
 static void mtk_mac_link_down(struct phylink_config *config, unsigned int mode,
 			      phy_interface_t interface)
 {
@@ -722,7 +690,6 @@ static void mtk_mac_link_up(struct phylink_config *config,
 
 static const struct phylink_mac_ops mtk_phylink_ops = {
 	.mac_select_pcs = mtk_mac_select_pcs,
-	.mac_pcs_get_state = mtk_mac_pcs_get_state,
 	.mac_config = mtk_mac_config,
 	.mac_finish = mtk_mac_finish,
 	.mac_link_down = mtk_mac_link_down,
@@ -4327,8 +4294,6 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 
 	mac->phylink_config.dev = &eth->netdev[id]->dev;
 	mac->phylink_config.type = PHYLINK_NETDEV;
-	/* This driver makes use of state->speed in mac_config */
-	mac->phylink_config.legacy_pre_march2020 = true;
 	mac->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
 		MAC_10 | MAC_100 | MAC_1000 | MAC_2500FD;
 
-- 
2.30.2