Hi Russell,
On Sat, Sep 23, 2023 at 04:48:57PM +0300, Vladimir Oltean wrote:
> Some phylink and phylib based systems might want to operate on backplane
> media types ("K" in the name), and thus, picking a phy_interface_t for
> them becomes a challenge.
>
> phy_interface_t is a description of the connection between the MAC and
> the PHY, or if a MAC-side PCS is present, the connection between that
> and the next link segment (which can be remote).
>
> A MAC-side PCS is so far considered to be a PCS handling link modes with
> optional C37 autoneg. But 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.
>
> (a) If we say that the PCS is MAC-side for C73 modes as well, the
> implication seems to be that the phy-mode should be one of
> PHY_INTERFACE_MODE_10GBASEKR, PHY_INTERFACE_MODE_1000BASEKX, etc.
> Similar to PHY_INTERFACE_MODE_1000BASEX which imitates the link mode
> ETHTOOL_LINK_MODE_1000baseX_Full_BIT.
>
> (b) If we say that the PCS is not MAC-side, but rather that the
> phylink_pcs represents an entire non-phylib backplane PHY which may
> negotiate one of many link modes (like a copper phylib PHY), then
> the phy-mode should probably be one of PHY_INTERFACE_MODE_XGMII,
> XLGMII etc. Or rather, because there is no MII pinout per se and the
> backplane PHY / phylink_pcs is internal, we can also use
> PHY_INTERFACE_MODE_INTERNAL.
>
> The trouble with (a), in my opinion, is that if we let the phy_interface_t
> follow the link mode like in the case of Base-X fiber modes, we have to
> consider the fact that C73 PHYs can advertise multiple link modes, so
> the phy_interface_t selection will be arbitrary, and any phy_interface_t
> selection will have to leave in the "supported" and "advertised" masks
> of link modes all the other backplane modes. This may be hard to justify.
>
> That is the reasoning based on which I selected this phy-mode to
> describe the setup in Layerscape SoCs which have integrated backplane
> autoneg support. The changes in phylink permit the managed =
> "in-band-status" fwnode property to be extended for C73 autoneg, which
> is then controllable through ethtool. With phy-mode = "internal" in an
> in-band autoneg mode, we advertise all backplane link modes. The list is
> not exhaustive and may be extended in the future.
>
> Link: https://lore.kernel.org/netdev/ZOXlpkbcAZ4okric@xxxxxxxxxxxxxxxxxxxxx/
> Link: https://lore.kernel.org/netdev/ZGIkGmyL8yL1q1zp@xxxxxxxxxxxxxxxxxxxxx/
> Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
> ---
> v1->v2: patch is new
>
> drivers/net/phy/phylink.c | 19 ++++++++++++++++++-
> include/linux/phylink.h | 1 +
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 548130d77302..88ace7e203c3 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -972,6 +972,21 @@ static int phylink_parse_mode(struct phylink *pl,
> phylink_set(pl->supported, 100000baseDR2_Full);
> break;
>
> + case PHY_INTERFACE_MODE_INTERNAL:
> + phylink_set(pl->supported, 1000baseKX_Full);
> + phylink_set(pl->supported, 10000baseKX4_Full);
> + phylink_set(pl->supported, 10000baseKR_Full);
> + phylink_set(pl->supported, 25000baseCR_Full);
> + phylink_set(pl->supported, 25000baseKR_Full);
> + phylink_set(pl->supported, 25000baseCR_S_Full);
> + phylink_set(pl->supported, 25000baseKR_S_Full);
> + phylink_set(pl->supported, 40000baseKR4_Full);
> + phylink_set(pl->supported, 50000baseKR2_Full);
> + phylink_set(pl->supported, 50000baseKR_Full);
> + phylink_set(pl->supported, 100000baseKR4_Full);
> + phylink_set(pl->supported, 100000baseKR2_Full);
> + break;
> +
> default:
> phylink_err(pl,
> "incorrect link mode %s for in-band status\n",
> @@ -1109,7 +1124,9 @@ static void phylink_mac_config(struct phylink *pl,
>
> static bool phylink_pcs_handles_an(phy_interface_t iface, unsigned int mode)
> {
> - return phy_interface_mode_is_8023z(iface) && phylink_autoneg_inband(mode);
> + return (phy_interface_mode_is_8023z(iface) ||
> + iface == PHY_INTERFACE_MODE_INTERNAL) &&
> + phylink_autoneg_inband(mode);
> }
>
> static void phylink_pcs_an_restart(struct phylink *pl)
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index 2b886ea654bb..7e8e26001587 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -141,6 +141,7 @@ static inline unsigned int phylink_pcs_neg_mode(unsigned int mode,
>
> case PHY_INTERFACE_MODE_1000BASEX:
> case PHY_INTERFACE_MODE_2500BASEX:
> + case PHY_INTERFACE_MODE_INTERNAL:
> /* 1000base-X is designed for use media-side for Fibre
> * connections, and thus the Autoneg bit needs to be
> * taken into account. We also do this for 2500base-X
> --
> 2.34.1
>
What do you think about this change?
