Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 3 Jun 2021 16:20:56 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>
Cc:     Jose.Abreu@synopsys.com, andrew@lunn.ch, hkallweit1@gmail.com,
        linux@armlinux.org.uk, kuba@kernel.org, netdev@vger.kernel.org,
        peppe.cavallaro@st.com, alexandre.torgue@foss.st.com,
        davem@davemloft.net, mcoquelin.stm32@gmail.com,
        weifeng.voon@intel.com, boon.leong.ong@intel.com,
        tee.min.tan@intel.com, vee.khee.wong@linux.intel.com,
        vee.khee.wong@intel.com, linux-stm32@st-md-mailman.stormreply.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [RESEND PATCH net-next v4 1/3] net: stmmac: split xPCS setup
 from mdio register
Message-ID: <20210603132056.zklgtbsslbkgqtsn@skbuf>
References: <20210603115032.2470-1-michael.wei.hong.sit@intel.com>
 <20210603115032.2470-2-michael.wei.hong.sit@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210603115032.2470-2-michael.wei.hong.sit@intel.com>
Precedence: bulk

Hi Michael,

On Thu, Jun 03, 2021 at 07:50:30PM +0800, Michael Sit Wei Hong wrote:
> From: Voon Weifeng <weifeng.voon@intel.com>
> 
> This patch is a preparation patch for the enabling of Intel mGbE 2.5Gbps
> link speed. The Intel mGbR link speed configuration (1G/2.5G) is depends on
> a mdio ADHOC register which can be configured in the bios menu.
> As PHY interface might be different for 1G and 2.5G, the mdio bus need be
> ready to check the link speed and select the PHY interface before probing
> the xPCS.
> 
> Signed-off-by: Voon Weifeng <weifeng.voon@intel.com>
> Signed-off-by: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  1 +
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c |  7 ++
>  .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 73 ++++++++++---------
>  3 files changed, 46 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index b6cd43eda7ac..fd7212afc543 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -311,6 +311,7 @@ enum stmmac_state {
>  int stmmac_mdio_unregister(struct net_device *ndev);
>  int stmmac_mdio_register(struct net_device *ndev);
>  int stmmac_mdio_reset(struct mii_bus *mii);
> +int stmmac_xpcs_setup(struct mii_bus *mii);
>  void stmmac_set_ethtool_ops(struct net_device *netdev);
>  
>  void stmmac_ptp_register(struct stmmac_priv *priv);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 13720bf6f6ff..eb81baeb13b0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7002,6 +7002,12 @@ int stmmac_dvr_probe(struct device *device,
>  		}
>  	}
>  
> +	if (priv->plat->mdio_bus_data->has_xpcs) {
> +		ret = stmmac_xpcs_setup(priv->mii);
> +		if (ret)
> +			goto error_xpcs_setup;
> +	}
> +

I don't understand why this change is necessary?

The XPCS probing code was at the end of stmmac_mdio_register().
You moved the code right _after_ stmmac_mdio_register().
So the code flow is exactly the same.

>  	ret = stmmac_phy_setup(priv);
>  	if (ret) {
>  		netdev_err(ndev, "failed to setup phy (%d)\n", ret);
> @@ -7038,6 +7044,7 @@ int stmmac_dvr_probe(struct device *device,
>  	unregister_netdev(ndev);
>  error_netdev_register:
>  	phylink_destroy(priv->phylink);
> +error_xpcs_setup:
>  error_phy_setup:
>  	if (priv->hw->pcs != STMMAC_PCS_TBI &&
>  	    priv->hw->pcs != STMMAC_PCS_RTBI)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> index e293bf1ce9f3..3bb0a787f136 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> @@ -397,6 +397,44 @@ int stmmac_mdio_reset(struct mii_bus *bus)
>  	return 0;
>  }
>  
> +int stmmac_xpcs_setup(struct mii_bus *bus)
> +{
> +	int mode, max_addr, addr, found, ret;
> +	struct net_device *ndev = bus->priv;
> +	struct mdio_xpcs_args *xpcs;
> +	struct stmmac_priv *priv;
> +
> +	priv = netdev_priv(ndev);
> +	xpcs = &priv->hw->xpcs_args;
> +	mode = priv->plat->phy_interface;
> +	max_addr = PHY_MAX_ADDR;
> +
> +	priv->hw->xpcs = mdio_xpcs_get_ops();
> +	if (!priv->hw->xpcs)
> +		return -ENODEV;
> +
> +	/* Try to probe the XPCS by scanning all addresses. */
> +	xpcs->bus = bus;
> +	found = 0;
> +
> +	for (addr = 0; addr < max_addr; addr++) {
> +		xpcs->addr = addr;
> +
> +		ret = stmmac_xpcs_probe(priv, xpcs, mode);
> +		if (!ret) {
> +			found = 1;
> +			break;
> +		}
> +	}
> +
> +	if (!found) {
> +		dev_warn(priv->device, "No xPCS found\n");
> +		return -ENODEV;
> +	}
> +
> +	return ret;
> +}
> +
>  /**
>   * stmmac_mdio_register
>   * @ndev: net device structure
> @@ -444,14 +482,6 @@ int stmmac_mdio_register(struct net_device *ndev)
>  		max_addr = PHY_MAX_ADDR;
>  	}
>  
> -	if (mdio_bus_data->has_xpcs) {
> -		priv->hw->xpcs = mdio_xpcs_get_ops();
> -		if (!priv->hw->xpcs) {
> -			err = -ENODEV;
> -			goto bus_register_fail;
> -		}
> -	}
> -
>  	if (mdio_bus_data->needs_reset)
>  		new_bus->reset = &stmmac_mdio_reset;
>  
> @@ -509,38 +539,11 @@ int stmmac_mdio_register(struct net_device *ndev)
>  		goto no_phy_found;
>  	}
>  
> -	/* Try to probe the XPCS by scanning all addresses. */
> -	if (priv->hw->xpcs) {
> -		struct mdio_xpcs_args *xpcs = &priv->hw->xpcs_args;
> -		int ret, mode = priv->plat->phy_interface;
> -		max_addr = PHY_MAX_ADDR;
> -
> -		xpcs->bus = new_bus;
> -
> -		found = 0;
> -		for (addr = 0; addr < max_addr; addr++) {
> -			xpcs->addr = addr;
> -
> -			ret = stmmac_xpcs_probe(priv, xpcs, mode);
> -			if (!ret) {
> -				found = 1;
> -				break;
> -			}
> -		}
> -
> -		if (!found && !mdio_node) {
> -			dev_warn(dev, "No XPCS found\n");
> -			err = -ENODEV;
> -			goto no_xpcs_found;
> -		}
> -	}
> -
>  bus_register_done:
>  	priv->mii = new_bus;
>  
>  	return 0;
>  
> -no_xpcs_found:
>  no_phy_found:
>  	mdiobus_unregister(new_bus);
>  bus_register_fail:
> -- 
> 2.17.1
>