Skip to content
Commit bcc096af authored by Vladimir Oltean's avatar Vladimir Oltean Committed by Xiaolei Wang
Browse files

net: phylink: add support for managed = "c73"

commit  437a7c67817908bf02b57f1259af2933ef9891e4 from
https://github.com/nxp-imx/linux-imx lf-6.1.y

Phylink requires modifications to properly support the clause 73 autoneg
concepts for the following reason.

A MAC-side PCS (modeled as phylink_pcs) is, so far, considered to be a
PCS (aka top-most layer of a non-phylib PHY) which handles link modes
with optional C37 autoneg (BASE-X, SGMII, USXGMII). Those link modes are
encoded within a phylib-specific phy_interface_t data type, which is
either fixed or changes at runtime based on a phylib PHY's decision, or
based on phylink_choose_sfp_interface(). C37 autoneg changes some of the
parameters of the established phy_interface_t (speed, duplex, flow
control).

As opposed to that, C73 autoneg (for backplanes and SFP28 modules)
is not at the same level in the OSI layering, so that existing model may
or may not apply. In addition to negotiating speed, duplex, flow control,
it also provides a standard way of exchanging the phy_interface_t
selection with the link partner as well.

Actually we already have backplane link modes present in enum
ethtool_link_mode_bit_indices already (a data type exposed to UAPI, and
common for all drivers, whether or not they use phylib or phylink or do
their own PHY management). I would have wished for it not to be
necessary to encode the backplane link modes in phy_interface_t as well,
since this constitutes a duplication with the ethtool link modes.

But it seems that phy_interface_t has become baked into phylink, as a
legacy from phylib. For example, we may have phylib PHYs that use
firmware to configure the system interface for C73 AN/LT with a fixed
link mode (see PHY_INTERFACE_MODE_10GKR in aqr107_read_status()).
Thus, 10GBase-KR is both an ethtool link mode (visible to the remote
end), as well as a phy_interface_t, visible to an attached phylib PHY
(completely optional within phylink).

A second reason for duplicating the backplane link modes into
phy_interface_t is the need to present a sane API to MAC drivers when
they use a C73-capable PCS. Currently, for non-C73, that API is already
phy_interface_t centric (state->interface).

The problem with state->interface within mac_config() and pcs_config()
is that it isn't valid, and we don't have a good placeholder for its
initial value, either (C73 can advertise multiple link modes, and until
autoneg is resolved, none of them is active).

Choices for state->interface like PHY_INTERFACE_MODE_XGMII, XLGMII or
even PHY_INTERFACE_MODE_INTERNAL make sense when imagining the
phylink_pcs as a complete (non-phylib) backplane PHY, but they don't
make sense when taking into consideration that an Aquantia phylib PHY
might see this phy_interface_t value.

So, the proposal is to modify phylink to derive the state->interface
from the resolved C73 ethtool link mode, and to pass that to
mac_link_up() and pcs_link_up().

The initial state->interface will bear less importance. It can either be
PHY_INTERFACE_MODE_NA, or the backplane link mode that the phylink_pcs
is preconfigured for. If the former, phylink will compute and fill in
the phy_interface_t corresponding to the highest-priority supported link
mode, anyway. What is important is that MAC drivers know that they need
to ignore state->interface at mac_config() time, and for that, some
API differentiation is required.

Introduce managed = "c73", which is like managed = "in-band-status"
except for the differences highlighted above between C37 and C73
autoneg.

pcs_validate(), pcs_get_state(), pcs_config(), pcs_enable(), all of
these need to know if the link operates in C73 or not. But that info
isn't available, and modifying all function prototypes is extremely
noisy. So as a very dirty hack, modify struct phylink_pcs to carry
information from the phylink core to the PCS driver, instead of passing
that info as an argument to the phylink_pcs callbacks. Whether a link
uses C73 or not is not supposed to change at runtime, so cfg_link_an_mode
should be enough.

Actually, what is even worse is that mac_select_pcs() takes the
phy_interface_t as input, and as mentioned, that data type is an output
of the C73 autoneg. So we cannot select a phylink_pcs without changes to
the mac_select_pcs() mechanism, either. Again, we should pass the
cfg_link_an_mode as an argument, but to be less noisy, we opt for
passing it through struct phylink_config instead.

Link: https://lore.kernel.org/netdev/ZOXlpkbcAZ4okric@shell.armlinux.org.uk/
Link: https://lore.kernel.org/netdev/ZGIkGmyL8yL1q1zp@shell.armlinux.org.uk/


Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: default avatarXiaolei Wang <xiaolei.wang@windriver.com>
parent 69394665
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment