Commit 9e10fb4c authored by Paolo Abeni's avatar Paolo Abeni
Browse files

Merge branch 'remove-legacy-phylink-behaviour' 

Russell King says:

====================
Remove legacy phylink behaviour

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.
====================

Link: https://lore.kernel.org/r/ZLw8DoRskRXLQK37@shell.armlinux.org.uk


Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parents ec87f054 c5714f68

drivers/net/ethernet/mediatek/mtk_eth_soc.c

+16 −78
Original line number Diff line number Diff line
@@ -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++)
@@ -602,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)

{
@@ -756,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,
@@ -4361,11 +4294,14 @@ 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;



	/* 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,
@@ -4373,6 +4309,7 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np) 


		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 +4782,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),

drivers/net/ethernet/mediatek/mtk_eth_soc.h

+1 −0
Original line number Diff line number Diff line
@@ -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;

drivers/net/phy/phylink.c

+19 −29
Original line number Diff line number Diff line
@@ -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);

		    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, state);

	pl->mac_ops->mac_config(pl->config, pl->cur_link_an_mode, &st);

}



static void phylink_pcs_an_restart(struct phylink *pl)
@@ -1198,13 +1205,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 +1257,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 +1489,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 +3503,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 +3553,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 +3664,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)

{

include/linux/phylink.h

+6 −39
Original line number Diff line number Diff line
@@ -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);