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

[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.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");
> 

Karthik/Ashok: Please retest it and reply.

Thanks,
Michal