| Message ID | 20201005142818.15110-1-claudiu.manoil@nxp.com |
|---|---|
| Headers | show |
| Series | enetc: Migrate to PHYLINK and PCS_LYNX | expand |
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.manoil@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
>-----Original Message----- >From: Ioana Ciornei <ioana.ciornei@nxp.com> >Sent: Tuesday, October 6, 2020 9:09 AM [...] > >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. > Agreed on the 2 findings. v2 sent. Thanks Ioana.