[PATCH] net: gem: Fix setting PCS auto-negotiation state

[Ramon Fried]

On Fri, Mar 12, 2021 at 12:56 AM Robert Hancock
<robert.hancock at calian.com> wrote:
>
> The code was trying to disable PCS auto-negotiation when a fixed-link node
> is present and enable it otherwise. However, the PCS registers were being
> written before the PCSSEL bit was set in the network configuration
> register, and it appears that in this state, PCS register writes are
> ignored. The result is that the intended change only took effect on the
> second network operation that was performed, since at that time PCSSEL is
> already enabled.
>
> Fix the order of register writes so that PCS registers are only written to
> after the PCS is enabled.
>
> Fixes: 26e62cc971 ("net: gem: Disable PCS autonegotiation in case of fixed-link")
>
> Signed-off-by: Robert Hancock <robert.hancock at calian.com>
> ---
>  drivers/net/zynq_gem.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
> index baf06a2ad8..ff59982267 100644
> --- a/drivers/net/zynq_gem.c
> +++ b/drivers/net/zynq_gem.c
> @@ -454,14 +454,6 @@ static int zynq_gem_init(struct udevice *dev)
>             priv->int_pcs) {
>                 nwconfig |= ZYNQ_GEM_NWCFG_SGMII_ENBL |
>                             ZYNQ_GEM_NWCFG_PCS_SEL;
> -#ifdef CONFIG_ARM64
> -       if (priv->phydev->phy_id != PHY_FIXED_ID)
> -               writel(readl(&regs->pcscntrl) | ZYNQ_GEM_PCS_CTL_ANEG_ENBL,
> -                      &regs->pcscntrl);
> -       else
> -               writel(readl(&regs->pcscntrl) & ~ZYNQ_GEM_PCS_CTL_ANEG_ENBL,
> -                      &regs->pcscntrl);
> -#endif
>         }
>
>         switch (priv->phydev->speed) {
> @@ -480,6 +472,23 @@ static int zynq_gem_init(struct udevice *dev)
>                 break;
>         }
>
> +#ifdef CONFIG_ARM64
> +       if (priv->interface == PHY_INTERFACE_MODE_SGMII &&
> +           priv->int_pcs) {
> +               /*
> +                * Disable AN for fixed link configuration, enable otherwise.
> +                * Must be written after PCS_SEL is set in nwconfig,
> +                * otherwise writes will not take effect.
> +                */
> +               if (priv->phydev->phy_id != PHY_FIXED_ID)
> +                       writel(readl(&regs->pcscntrl) | ZYNQ_GEM_PCS_CTL_ANEG_ENBL,
> +                              &regs->pcscntrl);
> +               else
> +                       writel(readl(&regs->pcscntrl) & ~ZYNQ_GEM_PCS_CTL_ANEG_ENBL,
> +                              &regs->pcscntrl);
> +       }
> +#endif
> +
>         ret = clk_set_rate(&priv->tx_clk, clk_rate);
>         if (IS_ERR_VALUE(ret)) {
>                 dev_err(dev, "failed to set tx clock rate\n");
> --
> 2.27.0
>
Reviewed-by: Ramon Fried <rfried.dev at gmail.com>