Skip to content

Commit 5092fb4

Browse files
committed
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/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
2 parents 9d922f5 + 1105304 commit 5092fb4

File tree

5 files changed

+37
-19
lines changed

5 files changed

+37
-19
lines changed

drivers/net/ethernet/atheros/ag71xx.c

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,17 +1024,6 @@ static void ag71xx_mac_config(struct phylink_config *config, unsigned int mode,
10241024
ag71xx_wr(ag, AG71XX_REG_FIFO_CFG3, ag->fifodata[2]);
10251025
}
10261026

1027-
static void ag71xx_mac_pcs_get_state(struct phylink_config *config,
1028-
struct phylink_link_state *state)
1029-
{
1030-
state->link = 0;
1031-
}
1032-
1033-
static void ag71xx_mac_an_restart(struct phylink_config *config)
1034-
{
1035-
/* Not Supported */
1036-
}
1037-
10381027
static void ag71xx_mac_link_down(struct phylink_config *config,
10391028
unsigned int mode, phy_interface_t interface)
10401029
{
@@ -1098,8 +1087,6 @@ static void ag71xx_mac_link_up(struct phylink_config *config,
10981087

10991088
static const struct phylink_mac_ops ag71xx_phylink_mac_ops = {
11001089
.validate = phylink_generic_validate,
1101-
.mac_pcs_get_state = ag71xx_mac_pcs_get_state,
1102-
.mac_an_restart = ag71xx_mac_an_restart,
11031090
.mac_config = ag71xx_mac_config,
11041091
.mac_link_down = ag71xx_mac_link_down,
11051092
.mac_link_up = ag71xx_mac_link_up,

drivers/net/ethernet/mediatek/mtk_eth_soc.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2923,6 +2923,10 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
29232923

29242924
mac->phylink_config.dev = &eth->netdev[id]->dev;
29252925
mac->phylink_config.type = PHYLINK_NETDEV;
2926+
/* This driver makes use of state->speed/state->duplex in
2927+
* mac_config
2928+
*/
2929+
mac->phylink_config.legacy_pre_march2020 = true;
29262930
mac->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
29272931
MAC_10 | MAC_100 | MAC_1000 | MAC_2500FD;
29282932

drivers/net/phy/phylink.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,7 @@ static void phylink_mac_pcs_an_restart(struct phylink *pl)
742742
phylink_autoneg_inband(pl->cur_link_an_mode)) {
743743
if (pl->pcs_ops)
744744
pl->pcs_ops->pcs_an_restart(pl->pcs);
745-
else
745+
else if (pl->config->legacy_pre_march2020)
746746
pl->mac_ops->mac_an_restart(pl->config);
747747
}
748748
}
@@ -803,7 +803,7 @@ static int phylink_change_inband_advert(struct phylink *pl)
803803
if (test_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state))
804804
return 0;
805805

806-
if (!pl->pcs_ops) {
806+
if (!pl->pcs_ops && pl->config->legacy_pre_march2020) {
807807
/* Legacy method */
808808
phylink_mac_config(pl, &pl->link_config);
809809
phylink_mac_pcs_an_restart(pl);
@@ -854,7 +854,8 @@ static void phylink_mac_pcs_get_state(struct phylink *pl,
854854

855855
if (pl->pcs_ops)
856856
pl->pcs_ops->pcs_get_state(pl->pcs, state);
857-
else if (pl->mac_ops->mac_pcs_get_state)
857+
else if (pl->mac_ops->mac_pcs_get_state &&
858+
pl->config->legacy_pre_march2020)
858859
pl->mac_ops->mac_pcs_get_state(pl->config, state);
859860
else
860861
state->link = 0;
@@ -1048,12 +1049,11 @@ static void phylink_resolve(struct work_struct *w)
10481049
}
10491050
phylink_major_config(pl, false, &link_state);
10501051
pl->link_config.interface = link_state.interface;
1051-
} else if (!pl->pcs_ops) {
1052+
} else if (!pl->pcs_ops && pl->config->legacy_pre_march2020) {
10521053
/* The interface remains unchanged, only the speed,
10531054
* duplex or pause settings have changed. Call the
10541055
* old mac_config() method to configure the MAC/PCS
1055-
* only if we do not have a PCS installed (an
1056-
* unconverted user.)
1056+
* only if we do not have a legacy MAC driver.
10571057
*/
10581058
phylink_mac_config(pl, &link_state);
10591059
}

include/linux/phylink.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ enum phylink_op_type {
8484
* struct phylink_config - PHYLINK configuration structure
8585
* @dev: a pointer to a struct device associated with the MAC
8686
* @type: operation type of PHYLINK instance
87+
* @legacy_pre_march2020: driver has not been updated for March 2020 updates
88+
* (See commit 7cceb599d15d ("net: phylink: avoid mac_config calls")
8789
* @pcs_poll: MAC PCS cannot provide link change interrupt
8890
* @poll_fixed_state: if true, starts link_poll,
8991
* if MAC link is at %MLO_AN_FIXED mode.
@@ -97,6 +99,7 @@ enum phylink_op_type {
9799
struct phylink_config {
98100
struct device *dev;
99101
enum phylink_op_type type;
102+
bool legacy_pre_march2020;
100103
bool pcs_poll;
101104
bool poll_fixed_state;
102105
bool ovr_an_inband;
@@ -187,6 +190,10 @@ void validate(struct phylink_config *config, unsigned long *supported,
187190
* negotiation completion state in @state->an_complete, and link up state
188191
* in @state->link. If possible, @state->lp_advertising should also be
189192
* populated.
193+
*
194+
* Note: This is a legacy method. This function will not be called unless
195+
* legacy_pre_march2020 is set in &struct phylink_config and there is no
196+
* PCS attached.
190197
*/
191198
void mac_pcs_get_state(struct phylink_config *config,
192199
struct phylink_link_state *state);
@@ -227,6 +234,15 @@ int mac_prepare(struct phylink_config *config, unsigned int mode,
227234
* guaranteed to be correct, and so any mac_config() implementation must
228235
* never reference these fields.
229236
*
237+
* Note: For legacy March 2020 drivers (drivers with legacy_pre_march2020 set
238+
* in their &phylnk_config and which don't have a PCS), this function will be
239+
* called on each link up event, and to also change the in-band advert. For
240+
* non-legacy drivers, it will only be called to reconfigure the MAC for a
241+
* "major" change in e.g. interface mode. It will not be called for changes
242+
* in speed, duplex or pause modes or to change the in-band advertisement.
243+
* In any case, it is strongly preferred that speed, duplex and pause settings
244+
* are handled in the mac_link_up() method and not in this method.
245+
*
230246
* (this requires a rewrite - please refer to mac_link_up() for situations
231247
* where the PCS and MAC are not tightly integrated.)
232248
*
@@ -311,6 +327,10 @@ int mac_finish(struct phylink_config *config, unsigned int mode,
311327
/**
312328
* mac_an_restart() - restart 802.3z BaseX autonegotiation
313329
* @config: a pointer to a &struct phylink_config.
330+
*
331+
* Note: This is a legacy method. This function will not be called unless
332+
* legacy_pre_march2020 is set in &struct phylink_config and there is no
333+
* PCS attached.
314334
*/
315335
void mac_an_restart(struct phylink_config *config);
316336

net/dsa/port.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,6 +1098,13 @@ int dsa_port_phylink_create(struct dsa_port *dp)
10981098
if (err)
10991099
mode = PHY_INTERFACE_MODE_NA;
11001100

1101+
/* Presence of phylink_mac_link_state or phylink_mac_an_restart is
1102+
* an indicator of a legacy phylink driver.
1103+
*/
1104+
if (ds->ops->phylink_mac_link_state ||
1105+
ds->ops->phylink_mac_an_restart)
1106+
dp->pl_config.legacy_pre_march2020 = true;
1107+
11011108
if (ds->ops->phylink_get_caps)
11021109
ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config);
11031110

0 commit comments

Comments
 (0)