Commit 5092fb44 authored by Jakub Kicinski's avatar Jakub Kicinski

Merge branch 'net-phylink-introduce-legacy-mode-flag'

Russell King says:

====================
net: phylink: introduce legacy mode flag

In March 2020, phylink gained support to split the PCS support out of
the MAC callbacks. By doing so, a slight behavioural difference was
introduced when a PCS is present, specifically:

1) the call to mac_config() when the link comes up or advertisement
   changes were eliminated
2) mac_an_restart() will never be called
3) mac_pcs_get_state() will never be called

The intention was to eventually remove this support once all phylink
users were converted. Unfortunately, this still hasn't happened - and
in some cases, it looks like it may never happen.

Through discussion with Sean Anderson, we now need to allow the PCS to
be optional for modern drivers, so we need a different way to identify
these legacy drivers - in that we wish to allow the "modern" behaviour
where mac_config() is not called on link-up events, even if there is
no PCS attached.

In order to do that, this series of patches introduce a
"legacy_pre_march2020" which is used to permit the old behaviour - in
other words, we get the old behaviour only when there is no PCS and
this flag is true. Otherwise, we get the new behaviour.

I decided to use the date of the change in the flag as just using
"legacy" or "legacy_driver" is too non-descript. An alternative could
be to use the git sha1 hash of the set of changes.

I believe I have added the legacy flag to all the drivers which use
legacy mode - that being the mtk_eth_soc ethernet driver, and many DSA
drivers - the ones which need the old behaviour are identified by
having non-NULL phylink_mac_link_state or phylink_mac_an_restart
methods in their dsa_switch_ops structure.

ag71xx and xilinx do not need the legacy flag. ag71xx is explained in
its own commit, and xilinx only updates the inband advertisement in
the mac_config() call, which is sufficient qualification to avoid it
being marked legacy.
====================

Link: https://lore.kernel.org/r/Ya+DGaGmGgWrlVkW@shell.armlinux.org.ukSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 9d922f5d 11053047
...@@ -1024,17 +1024,6 @@ static void ag71xx_mac_config(struct phylink_config *config, unsigned int mode, ...@@ -1024,17 +1024,6 @@ static void ag71xx_mac_config(struct phylink_config *config, unsigned int mode,
ag71xx_wr(ag, AG71XX_REG_FIFO_CFG3, ag->fifodata[2]); ag71xx_wr(ag, AG71XX_REG_FIFO_CFG3, ag->fifodata[2]);
} }
static void ag71xx_mac_pcs_get_state(struct phylink_config *config,
struct phylink_link_state *state)
{
state->link = 0;
}
static void ag71xx_mac_an_restart(struct phylink_config *config)
{
/* Not Supported */
}
static void ag71xx_mac_link_down(struct phylink_config *config, static void ag71xx_mac_link_down(struct phylink_config *config,
unsigned int mode, phy_interface_t interface) unsigned int mode, phy_interface_t interface)
{ {
...@@ -1098,8 +1087,6 @@ static void ag71xx_mac_link_up(struct phylink_config *config, ...@@ -1098,8 +1087,6 @@ static void ag71xx_mac_link_up(struct phylink_config *config,
static const struct phylink_mac_ops ag71xx_phylink_mac_ops = { static const struct phylink_mac_ops ag71xx_phylink_mac_ops = {
.validate = phylink_generic_validate, .validate = phylink_generic_validate,
.mac_pcs_get_state = ag71xx_mac_pcs_get_state,
.mac_an_restart = ag71xx_mac_an_restart,
.mac_config = ag71xx_mac_config, .mac_config = ag71xx_mac_config,
.mac_link_down = ag71xx_mac_link_down, .mac_link_down = ag71xx_mac_link_down,
.mac_link_up = ag71xx_mac_link_up, .mac_link_up = ag71xx_mac_link_up,
......
...@@ -2923,6 +2923,10 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np) ...@@ -2923,6 +2923,10 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
mac->phylink_config.dev = &eth->netdev[id]->dev; mac->phylink_config.dev = &eth->netdev[id]->dev;
mac->phylink_config.type = PHYLINK_NETDEV; mac->phylink_config.type = PHYLINK_NETDEV;
/* This driver makes use of state->speed/state->duplex in
* mac_config
*/
mac->phylink_config.legacy_pre_march2020 = true;
mac->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | mac->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
MAC_10 | MAC_100 | MAC_1000 | MAC_2500FD; MAC_10 | MAC_100 | MAC_1000 | MAC_2500FD;
......
...@@ -742,7 +742,7 @@ static void phylink_mac_pcs_an_restart(struct phylink *pl) ...@@ -742,7 +742,7 @@ static void phylink_mac_pcs_an_restart(struct phylink *pl)
phylink_autoneg_inband(pl->cur_link_an_mode)) { phylink_autoneg_inband(pl->cur_link_an_mode)) {
if (pl->pcs_ops) if (pl->pcs_ops)
pl->pcs_ops->pcs_an_restart(pl->pcs); pl->pcs_ops->pcs_an_restart(pl->pcs);
else else if (pl->config->legacy_pre_march2020)
pl->mac_ops->mac_an_restart(pl->config); pl->mac_ops->mac_an_restart(pl->config);
} }
} }
...@@ -803,7 +803,7 @@ static int phylink_change_inband_advert(struct phylink *pl) ...@@ -803,7 +803,7 @@ static int phylink_change_inband_advert(struct phylink *pl)
if (test_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state)) if (test_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state))
return 0; return 0;
if (!pl->pcs_ops) { if (!pl->pcs_ops && pl->config->legacy_pre_march2020) {
/* Legacy method */ /* Legacy method */
phylink_mac_config(pl, &pl->link_config); phylink_mac_config(pl, &pl->link_config);
phylink_mac_pcs_an_restart(pl); phylink_mac_pcs_an_restart(pl);
...@@ -854,7 +854,8 @@ static void phylink_mac_pcs_get_state(struct phylink *pl, ...@@ -854,7 +854,8 @@ static void phylink_mac_pcs_get_state(struct phylink *pl,
if (pl->pcs_ops) if (pl->pcs_ops)
pl->pcs_ops->pcs_get_state(pl->pcs, state); pl->pcs_ops->pcs_get_state(pl->pcs, state);
else if (pl->mac_ops->mac_pcs_get_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); pl->mac_ops->mac_pcs_get_state(pl->config, state);
else else
state->link = 0; state->link = 0;
...@@ -1048,12 +1049,11 @@ static void phylink_resolve(struct work_struct *w) ...@@ -1048,12 +1049,11 @@ static void phylink_resolve(struct work_struct *w)
} }
phylink_major_config(pl, false, &link_state); phylink_major_config(pl, false, &link_state);
pl->link_config.interface = link_state.interface; pl->link_config.interface = link_state.interface;
} else if (!pl->pcs_ops) { } else if (!pl->pcs_ops && pl->config->legacy_pre_march2020) {
/* The interface remains unchanged, only the speed, /* The interface remains unchanged, only the speed,
* duplex or pause settings have changed. Call the * duplex or pause settings have changed. Call the
* old mac_config() method to configure the MAC/PCS * old mac_config() method to configure the MAC/PCS
* only if we do not have a PCS installed (an * only if we do not have a legacy MAC driver.
* unconverted user.)
*/ */
phylink_mac_config(pl, &link_state); phylink_mac_config(pl, &link_state);
} }
......
...@@ -84,6 +84,8 @@ enum phylink_op_type { ...@@ -84,6 +84,8 @@ enum phylink_op_type {
* struct phylink_config - PHYLINK configuration structure * struct phylink_config - PHYLINK configuration structure
* @dev: a pointer to a struct device associated with the MAC * @dev: a pointer to a struct device associated with the MAC
* @type: operation type of PHYLINK instance * @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")
* @pcs_poll: MAC PCS cannot provide link change interrupt * @pcs_poll: MAC PCS cannot provide link change interrupt
* @poll_fixed_state: if true, starts link_poll, * @poll_fixed_state: if true, starts link_poll,
* if MAC link is at %MLO_AN_FIXED mode. * if MAC link is at %MLO_AN_FIXED mode.
...@@ -97,6 +99,7 @@ enum phylink_op_type { ...@@ -97,6 +99,7 @@ enum phylink_op_type {
struct phylink_config { struct phylink_config {
struct device *dev; struct device *dev;
enum phylink_op_type type; enum phylink_op_type type;
bool legacy_pre_march2020;
bool pcs_poll; bool pcs_poll;
bool poll_fixed_state; bool poll_fixed_state;
bool ovr_an_inband; bool ovr_an_inband;
...@@ -187,6 +190,10 @@ void validate(struct phylink_config *config, unsigned long *supported, ...@@ -187,6 +190,10 @@ void validate(struct phylink_config *config, unsigned long *supported,
* negotiation completion state in @state->an_complete, and link up state * negotiation completion state in @state->an_complete, and link up state
* in @state->link. If possible, @state->lp_advertising should also be * in @state->link. If possible, @state->lp_advertising should also be
* populated. * 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, void mac_pcs_get_state(struct phylink_config *config,
struct phylink_link_state *state); struct phylink_link_state *state);
...@@ -227,6 +234,15 @@ int mac_prepare(struct phylink_config *config, unsigned int mode, ...@@ -227,6 +234,15 @@ int mac_prepare(struct phylink_config *config, unsigned int mode,
* guaranteed to be correct, and so any mac_config() implementation must * guaranteed to be correct, and so any mac_config() implementation must
* never reference these fields. * 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 * (this requires a rewrite - please refer to mac_link_up() for situations
* where the PCS and MAC are not tightly integrated.) * where the PCS and MAC are not tightly integrated.)
* *
...@@ -311,6 +327,10 @@ int mac_finish(struct phylink_config *config, unsigned int mode, ...@@ -311,6 +327,10 @@ int mac_finish(struct phylink_config *config, unsigned int mode,
/** /**
* mac_an_restart() - restart 802.3z BaseX autonegotiation * mac_an_restart() - restart 802.3z BaseX autonegotiation
* @config: a pointer to a &struct phylink_config. * @config: a pointer to a &struct phylink_config.
*
* 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_an_restart(struct phylink_config *config); void mac_an_restart(struct phylink_config *config);
......
...@@ -1098,6 +1098,13 @@ int dsa_port_phylink_create(struct dsa_port *dp) ...@@ -1098,6 +1098,13 @@ int dsa_port_phylink_create(struct dsa_port *dp)
if (err) if (err)
mode = PHY_INTERFACE_MODE_NA; mode = PHY_INTERFACE_MODE_NA;
/* Presence of phylink_mac_link_state or phylink_mac_an_restart is
* an indicator of a legacy phylink driver.
*/
if (ds->ops->phylink_mac_link_state ||
ds->ops->phylink_mac_an_restart)
dp->pl_config.legacy_pre_march2020 = true;
if (ds->ops->phylink_get_caps) if (ds->ops->phylink_get_caps)
ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config); ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment