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

[Michal Simek]

On 3/11/21 11:55 PM, Robert Hancock 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.hanc...@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");
> 

Applied.
M