On Mon, Oct 05, 2020 at 05:28:18PM +0300, Claudiu Manoil wrote:
> This is a methodical transition of the driver from phylib
> to phylink, following the guidelines from sfp-phylink.rst.
> The MAC register configurations based on interface mode
> were moved from the probing path to the mac_config() hook.
> MAC enable and disable commands (enabling Rx and Tx paths
> at MAC level) were also extracted and assigned to their
> corresponding phylink hooks.
> As part of the migration to phylink, the serdes configuration
> from the driver was offloaded to the PCS_LYNX module,
> introduced in commit 0da4c3d393e4 ("net: phy: add Lynx PCS module"),
> the PCS_LYNX module being a mandatory component required to
> make the enetc driver work with phylink.
>
> Signed-off-by: Claudiu Manoil <claudiu.man...@nxp.com>
> ---
> drivers/net/ethernet/freescale/enetc/Kconfig | 5 +-
> drivers/net/ethernet/freescale/enetc/enetc.c | 53 ++--
> drivers/net/ethernet/freescale/enetc/enetc.h | 9 +-
> .../ethernet/freescale/enetc/enetc_ethtool.c | 26 +-
> .../net/ethernet/freescale/enetc/enetc_pf.c | 246 +++++++++---------
> .../net/ethernet/freescale/enetc/enetc_pf.h | 8 +-
> .../net/ethernet/freescale/enetc/enetc_qos.c | 9 +-
> 7 files changed, 190 insertions(+), 166 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/enetc/Kconfig
> b/drivers/net/ethernet/freescale/enetc/Kconfig
> index 37b804f8bd76..0fa18b00c49b 100644
> --- a/drivers/net/ethernet/freescale/enetc/Kconfig
> +++ b/drivers/net/ethernet/freescale/enetc/Kconfig
(...)
> @@ -922,61 +889,118 @@ static void enetc_mdiobus_destroy(struct enetc_pf *pf)
> enetc_imdio_remove(pf);
> }
>
> -static void enetc_configure_sgmii(struct phy_device *pcs)
> +static void enetc_pl_mac_validate(struct phylink_config *config,
> + unsigned long *supported,
> + struct phylink_link_state *state)
> {
> - /* SGMII spec requires tx_config_Reg[15:0] to be exactly 0x4001
> - * for the MAC PCS in order to acknowledge the AN.
> - */
> - phy_write(pcs, MII_ADVERTISE, ADVERTISE_SGMII | ADVERTISE_LPACK);
> + struct enetc_pf *pf = phylink_to_enetc_pf(config);
> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> +
> + if (state->interface != PHY_INTERFACE_MODE_NA &&
> + state->interface != pf->if_mode) {
> + bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> + return;
> + }
>
> - phy_write(pcs, ENETC_PCS_IF_MODE,
> - ENETC_PCS_IF_MODE_SGMII_EN |
> - ENETC_PCS_IF_MODE_USE_SGMII_AN);
> + phylink_set_port_modes(mask);
> + phylink_set(mask, Autoneg);
> + phylink_set(mask, Pause);
> + phylink_set(mask, Asym_Pause);
> + phylink_set(mask, 10baseT_Half);
> + phylink_set(mask, 10baseT_Full);
> + phylink_set(mask, 100baseT_Half);
> + phylink_set(mask, 100baseT_Full);
> + phylink_set(mask, 100baseT_Half);
> + phylink_set(mask, 1000baseT_Half);
> + phylink_set(mask, 1000baseT_Full);
> +
> + if (state->interface == PHY_INTERFACE_MODE_INTERNAL ||
> + state->interface == PHY_INTERFACE_MODE_2500BASEX ||
> + state->interface == PHY_INTERFACE_MODE_USXGMII) {
> + phylink_set(mask, 2500baseT_Full);
> + phylink_set(mask, 2500baseX_Full);
> + }
Shouldn't the driver reject any interface mode which it does not
support? Either here in the validate callback or directly
where the pf->if_mode is set.
>
> - /* Adjust link timer for SGMII */
> - phy_write(pcs, ENETC_PCS_LINK_TIMER1, ENETC_PCS_LINK_TIMER1_VAL);
> - phy_write(pcs, ENETC_PCS_LINK_TIMER2, ENETC_PCS_LINK_TIMER2_VAL);
> + bitmap_and(supported, supported, mask,
> + __ETHTOOL_LINK_MODE_MASK_NBITS);
> + bitmap_and(state->advertising, state->advertising, mask,
> + __ETHTOOL_LINK_MODE_MASK_NBITS);
> +}
>
(...)
>
> -static void enetc_configure_serdes(struct enetc_pf *pf)
> +static const struct phylink_mac_ops enetc_mac_phylink_ops = {
> + .validate = enetc_pl_mac_validate,
> + .mac_config = enetc_pl_mac_config,
> + .mac_link_up = enetc_pl_mac_link_up,
> + .mac_link_down = enetc_pl_mac_link_down,
> +};
> +
> +static int enetc_phylink_create(struct enetc_ndev_priv *priv)
> {
> - switch (pf->if_mode) {
> - case PHY_INTERFACE_MODE_SGMII:
> - enetc_configure_sgmii(pf->pcs);
> - break;
> - case PHY_INTERFACE_MODE_2500BASEX:
> - enetc_configure_2500basex(pf->pcs);
> - break;
> - case PHY_INTERFACE_MODE_USXGMII:
> - enetc_configure_usxgmii(pf->pcs);
> - break;
> - default:
> - dev_dbg(&pf->si->pdev->dev, "Unsupported link mode %s\n",
> - phy_modes(pf->if_mode));
> + struct enetc_pf *pf = enetc_si_priv(priv->si);
> + struct device *dev = &pf->si->pdev->dev;
> + struct phylink *phylink;
> + int err;
> +
> + pf->phylink_config.dev = &priv->ndev->dev;
> + pf->phylink_config.type = PHYLINK_NETDEV;
> + if (enetc_port_has_pcs(pf))
> + pf->phylink_config.pcs_poll = true;
> +
There is no need for the pcs_poll to be set by the driver, the lynx PCS
module will set it up for you.
Ioana
