lkml.org 
[lkml]   [2022]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH net-next 2/2] net: altera: tse: convert to phylink
On Tue, Aug 23, 2022 at 04:05:17PM +0200, Maxime Chevallier wrote:
> This commit converts the Altera Triple Speed Ethernet Controller to
> phylink. This controller supports MII, GMII and RGMII with its MAC, and
> SGMII + 1000BaseX through a small embedded PCS.
>
> The PCS itself has a register set very similar to what is found in a
> typical 802.3 ethernet PHY, but this register set memory-mapped instead
> of lying on an mdio bus.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

This needs some work.

> +static void alt_tse_mac_link_state(struct phylink_config *config,
> + struct phylink_link_state *state)
> +{
> + struct net_device *ndev = to_net_dev(config->dev);
> + struct altera_tse_private *priv = netdev_priv(ndev);
> +
> + u16 bmsr, lpa;
> +
> + bmsr = sgmii_pcs_read(priv, MII_BMSR);
> + lpa = sgmii_pcs_read(priv, MII_LPA);
> +
> + phylink_mii_c22_pcs_decode_state(state, bmsr, lpa);
> +}
> +
> +static void alt_tse_mac_an_restart(struct phylink_config *config)
> +{
> + struct net_device *ndev = to_net_dev(config->dev);
> + struct altera_tse_private *priv = netdev_priv(ndev);
> + u16 bmcr;
> +
> + bmcr = sgmii_pcs_read(priv, MII_BMCR);
> + bmcr |= BMCR_ANRESTART;
> + sgmii_pcs_write(priv, MII_BMCR, bmcr);
> +}
> +
> +static void alt_tse_pcs_config(struct net_device *ndev,
> + const struct phylink_link_state *state)
> +{
> + struct altera_tse_private *priv = netdev_priv(ndev);
> + u32 ctrl, if_mode;
> +
> + if (state->interface != PHY_INTERFACE_MODE_SGMII &&
> + state->interface != PHY_INTERFACE_MODE_1000BASEX)
> + return;
> +
> + ctrl = sgmii_pcs_read(priv, MII_BMCR);
> + if_mode = sgmii_pcs_read(priv, SGMII_PCS_IF_MODE);
> +
> + /* Set link timer to 1.6ms, as per the MegaCore Function User Guide */
> + sgmii_pcs_write(priv, SGMII_PCS_LINK_TIMER_0, 0x0D40);
> + sgmii_pcs_write(priv, SGMII_PCS_LINK_TIMER_1, 0x03);
> +
> + if (state->interface == PHY_INTERFACE_MODE_SGMII) {
> + if_mode |= PCS_IF_MODE_USE_SGMII_AN | PCS_IF_MODE_SGMII_ENA;
> + } else if (state->interface == PHY_INTERFACE_MODE_1000BASEX) {
> + if_mode &= ~(PCS_IF_MODE_USE_SGMII_AN | PCS_IF_MODE_SGMII_ENA);
> + if_mode |= PCS_IF_MODE_SGMI_SPEED_1000;
> + }
> +
> + ctrl |= (BMCR_SPEED1000 | BMCR_FULLDPLX | BMCR_ANENABLE);
> +
> + sgmii_pcs_write(priv, MII_BMCR, ctrl);
> + sgmii_pcs_write(priv, SGMII_PCS_IF_MODE, if_mode);
> +
> + sgmii_pcs_reset(priv);
> +}

These look like they can be plugged directly into the phylink_pcs
support - please use that in preference to bolting it ino the MAC
ops - as every other ethernet driver (with the exception of
mtk_eth_soc) does today.

> +
> +static void alt_tse_mac_config(struct phylink_config *config, unsigned int mode,
> + const struct phylink_link_state *state)
> +{
> + struct net_device *ndev = to_net_dev(config->dev);
> + struct altera_tse_private *priv = netdev_priv(ndev);
> + u32 ctrl;
> +
> + ctrl = csrrd32(priv->mac_dev, tse_csroffs(command_config));
> + ctrl &= ~(MAC_CMDCFG_ENA_10 | MAC_CMDCFG_ETH_SPEED | MAC_CMDCFG_HD_ENA);
> +
> + if (state->duplex == DUPLEX_HALF)
> + ctrl |= MAC_CMDCFG_HD_ENA;

Using state->duplex in mac_config has always been a problem, it's not
well defined, and there are paths through phylink where state->duplex
does not reflect the state of the link when this function is called.
This is why it's always been clearly documented that this shall not be
used in mac_config.

> +
> + if (state->speed == SPEED_1000)
> + ctrl |= MAC_CMDCFG_ETH_SPEED;
> + else if (state->speed == SPEED_10)
> + ctrl |= MAC_CMDCFG_ENA_10;

Using state->speed brings with it the same problems as state->duplex.

Please instead use mac_link_up() (and pcs_link_up()) - which are the
only callbacks from phylink that you can be sure give you the
speed, duplex and pause settings for the link.

Thanks.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

\
 
 \ /
  Last update: 2022-08-23 21:21    [W:0.064 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site