Skip to content

Commit

Permalink
Merge branch 'improve-clause-45-support-in-phylink'
Browse files Browse the repository at this point in the history 
Russell King says:

====================
improve clause 45 support in phylink

These three patches improve the clause 45 support in phylink, fixing
some corner cases that have been noticed with the addition of SFP+
NBASE-T modules, but are actually a little more wisespread than I
initially realised.

The first issue was spotted with a NBASE-T PHY on a SFP+ module plugged
into a mvneta platform. When these PHYs are not operating in USXGMII
mode, but are in a single-lane Serdes mode, they will switch between
one of several different PHY interface modes.

If we call the MAC validate() function with the current PHY interface
mode, we will restrict the supported and advertising masks to the link
modes that the current PHY interface mode supports. For example, if we
determine that we want to start the PHY with an interface mode of
2500BASE-X, then this setup will restrict the advertisement and
supported masks to 2.5G speed link modes.

What we actually want for these PHYs is to allow them to support any
link modes that the PHY supports _and_ the MAC is also capable of
supporting. Without knowing the details of the PHY interface modes that
may be used, we can do this by using PHY_INTERFACE_MODE_NA to validate
and restrict the link modes to any that the MAC supports.

mvpp2 with the 88X3310 PHY avoids this problem, because the validate()
implementation allows all MAC supported speeds not only for
PHY_INTERFACE_MODE_NA, but also for XAUI and 10GKR modes.

The first patch addresses this; current MAC drivers should continue to
work as-is, but there will be a follow-on patch to fixup at least
mvpp2.

The second issue addresses a very similar problem that occurs when
trying to use ethtool to alter the advertisement mask - we call
the MAC validate() function with the current interface mode, the
current support and requested advertisement masks. This immediately
restricts the advertisement in the same way as the above.

This patch series addresses both issues, although the patches are not
in the above order.

v2: fix patch 3 missing 1G link modes for SGMII and RGMII interface
    modes.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Dec 17, 2019
2 parents 6f6dded + ef8e0b8 commit 4e133f7
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 44 deletions.
22 changes: 18 additions & 4 deletions drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4792,6 +4792,8 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
phylink_set(mask, 10000baseER_Full);
phylink_set(mask, 10000baseKR_Full);
}
if (state->interface != PHY_INTERFACE_MODE_NA)
break;
/* Fall-through */
case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_ID:
Expand All @@ -4802,13 +4804,23 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
phylink_set(mask, 10baseT_Full);
phylink_set(mask, 100baseT_Half);
phylink_set(mask, 100baseT_Full);
phylink_set(mask, 1000baseT_Full);
phylink_set(mask, 1000baseX_Full);
if (state->interface != PHY_INTERFACE_MODE_NA)
break;
/* Fall-through */
case PHY_INTERFACE_MODE_1000BASEX:
case PHY_INTERFACE_MODE_2500BASEX:
phylink_set(mask, 1000baseT_Full);
phylink_set(mask, 1000baseX_Full);
phylink_set(mask, 2500baseT_Full);
phylink_set(mask, 2500baseX_Full);
if (port->comphy ||
state->interface != PHY_INTERFACE_MODE_2500BASEX) {
phylink_set(mask, 1000baseT_Full);
phylink_set(mask, 1000baseX_Full);
}
if (port->comphy ||
state->interface == PHY_INTERFACE_MODE_2500BASEX) {
phylink_set(mask, 2500baseT_Full);
phylink_set(mask, 2500baseX_Full);
}
break;
default:
goto empty_set;
Expand All @@ -4817,6 +4829,8 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
bitmap_and(supported, supported, mask, __ETHTOOL_LINK_MODE_MASK_NBITS);
bitmap_and(state->advertising, state->advertising, mask,
__ETHTOOL_LINK_MODE_MASK_NBITS);

phylink_helper_basex_speed(state);
return;

empty_set:
Expand Down
106 changes: 66 additions & 40 deletions drivers/net/phy/phylink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,19 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy,
memset(&config, 0, sizeof(config));
linkmode_copy(supported, phy->supported);
linkmode_copy(config.advertising, phy->advertising);
config.interface = interface;

/* Clause 45 PHYs switch their Serdes lane between several different
* modes, normally 10GBASE-R, SGMII. Some use 2500BASE-X for 2.5G
* speeds. We really need to know which interface modes the PHY and
* MAC supports to properly work out which linkmodes can be supported.
*/
if (phy->is_c45 &&
interface != PHY_INTERFACE_MODE_RXAUI &&
interface != PHY_INTERFACE_MODE_XAUI &&
interface != PHY_INTERFACE_MODE_USXGMII)
config.interface = PHY_INTERFACE_MODE_NA;
else
config.interface = interface;

ret = phylink_validate(pl, supported, &config);
if (ret)
Expand Down Expand Up @@ -1230,44 +1242,66 @@ int phylink_ethtool_ksettings_set(struct phylink *pl,
__set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, config.advertising);
}

if (phylink_validate(pl, support, &config))
return -EINVAL;

/* If autonegotiation is enabled, we must have an advertisement */
if (config.an_enabled && phylink_is_empty_linkmode(config.advertising))
return -EINVAL;

our_kset = *kset;
linkmode_copy(our_kset.link_modes.advertising, config.advertising);
our_kset.base.speed = config.speed;
our_kset.base.duplex = config.duplex;

/* If we have a PHY, configure the phy */
if (pl->phydev) {
/* If we have a PHY, we process the kset change via phylib.
* phylib will call our link state function if the PHY
* parameters have changed, which will trigger a resolve
* and update the MAC configuration.
*/
our_kset = *kset;
linkmode_copy(our_kset.link_modes.advertising,
config.advertising);
our_kset.base.speed = config.speed;
our_kset.base.duplex = config.duplex;

ret = phy_ethtool_ksettings_set(pl->phydev, &our_kset);
if (ret)
return ret;
}

mutex_lock(&pl->state_mutex);
/* Configure the MAC to match the new settings */
linkmode_copy(pl->link_config.advertising, our_kset.link_modes.advertising);
pl->link_config.interface = config.interface;
pl->link_config.speed = our_kset.base.speed;
pl->link_config.duplex = our_kset.base.duplex;
pl->link_config.an_enabled = our_kset.base.autoneg != AUTONEG_DISABLE;
mutex_lock(&pl->state_mutex);
/* Save the new configuration */
linkmode_copy(pl->link_config.advertising,
our_kset.link_modes.advertising);
pl->link_config.interface = config.interface;
pl->link_config.speed = our_kset.base.speed;
pl->link_config.duplex = our_kset.base.duplex;
pl->link_config.an_enabled = our_kset.base.autoneg !=
AUTONEG_DISABLE;
mutex_unlock(&pl->state_mutex);
} else {
/* For a fixed link, this isn't able to change any parameters,
* which just leaves inband mode.
*/
if (phylink_validate(pl, support, &config))
return -EINVAL;

/* If we have a PHY, phylib will call our link state function if the
* mode has changed, which will trigger a resolve and update the MAC
* configuration. For a fixed link, this isn't able to change any
* parameters, which just leaves inband mode.
*/
if (pl->cur_link_an_mode == MLO_AN_INBAND &&
!test_bit(PHYLINK_DISABLE_STOPPED, &pl->phylink_disable_state)) {
phylink_mac_config(pl, &pl->link_config);
phylink_mac_an_restart(pl);
/* If autonegotiation is enabled, we must have an advertisement */
if (config.an_enabled &&
phylink_is_empty_linkmode(config.advertising))
return -EINVAL;

mutex_lock(&pl->state_mutex);
linkmode_copy(pl->link_config.advertising, config.advertising);
pl->link_config.interface = config.interface;
pl->link_config.speed = config.speed;
pl->link_config.duplex = config.duplex;
pl->link_config.an_enabled = kset->base.autoneg !=
AUTONEG_DISABLE;

if (pl->cur_link_an_mode == MLO_AN_INBAND &&
!test_bit(PHYLINK_DISABLE_STOPPED,
&pl->phylink_disable_state)) {
/* If in 802.3z mode, this updates the advertisement.
*
* If we are in SGMII mode without a PHY, there is no
* advertisement; the only thing we have is the pause
* modes which can only come from a PHY.
*/
phylink_mac_config(pl, &pl->link_config);
phylink_mac_an_restart(pl);
}
mutex_unlock(&pl->state_mutex);
}
mutex_unlock(&pl->state_mutex);

return 0;
}
Expand Down Expand Up @@ -1880,14 +1914,6 @@ static int phylink_sfp_connect_phy(void *upstream, struct phy_device *phy)
if (ret < 0)
return ret;

/* Clause 45 PHYs switch their Serdes lane between several different
* modes, normally 10GBASE-R, SGMII. Some use 2500BASE-X for 2.5G
* speeds. We really need to know which interface modes the PHY and
* MAC supports to properly work out which linkmodes can be supported.
*/
if (phy->is_c45)
interface = PHY_INTERFACE_MODE_NA;

ret = phylink_bringup_phy(pl, phy, interface);
if (ret)
phy_detach(phy);
Expand Down

0 comments on commit 4e133f7

Please sign in to comment.