From: Ioana Ciornei <ioana.ciornei@nxp.com>
To: Claudiu Manoil <claudiu.manoil@nxp.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"David S . Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Russell King <linux@armlinux.org.uk>,
Vladimir Oltean <vladimir.oltean@nxp.com>
Subject: Re: [PATCH net-next 4/4] enetc: Migrate to PHYLINK and PCS_LYNX
Date: Tue, 6 Oct 2020 06:09:00 +0000 [thread overview]
Message-ID: <20201006060859.hapkjgcv4pwzhkrv@skbuf> (raw)
In-Reply-To: <20201005142818.15110-5-claudiu.manoil@nxp.com>
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
next prev parent reply other threads:[~2020-10-06 6:09 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-05 14:28 [PATCH net-next 0/4] enetc: Migrate to PHYLINK and PCS_LYNX Claudiu Manoil
2020-10-05 14:28 ` [PATCH net-next 1/4] enetc: Clean up MAC and link configuration Claudiu Manoil
2020-10-05 14:28 ` [PATCH net-next 2/4] enetc: Clean up serdes configuration Claudiu Manoil
2020-10-05 14:28 ` [PATCH net-next 3/4] arm64: dts: fsl-ls1028a-rdb: Specify in-band mode for ENETC port 0 Claudiu Manoil
2020-10-05 14:28 ` [PATCH net-next 4/4] enetc: Migrate to PHYLINK and PCS_LYNX Claudiu Manoil
2020-10-06 6:09 ` Ioana Ciornei [this message]
2020-10-07 10:06 ` Claudiu Manoil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201006060859.hapkjgcv4pwzhkrv@skbuf \
--to=ioana.ciornei@nxp.com \
--cc=claudiu.manoil@nxp.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=vladimir.oltean@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).