[PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS

Patrick DELAUNAY patrick.delaunay at foss.st.com
Mon Nov 15 11:41:30 CET 2021


Hi,

On 11/10/21 6:42 AM, Joakim Zhang wrote:
> For EQOS ethernet, it will do phy_connect() and phy_config() when start
> the ethernet (eqos_srart()), users need wait seconds for PHY auto negotiation

s/eqos_srart()/eqos_start()/


> to complete when do tftp boot.
>      phy_config()
>              -> board_phy_config()
>                      -> phydev->drv->config() // i.MX8MP/DXL use realtek PHY
>                              -> rtl8211f_config()
>                                      -> genphy_config_aneg()
>                                              -> genphy_restart_aneg() // restart auto-nego, then need seconds to complete
>
> To avoid wasting time, we can move PHY connection and configuration from
> eqos_start() to eqos_probe(). This patch also moves clock setting from
> eqos_start() to eqos_probe() as MDIO access need CSR clock, there is no
> function change.
>
> Tested-on: i.MX8MP & i.MX8DXL
>
> Before:
> u-boot=> run netboot
> Booting from net ...
> ethernet at 30bf0000 Waiting for PHY auto negotiation to complete....... done
> BOOTP broadcast 1
> DHCP client bound to address 10.193.102.192 (313 ms)
> Using ethernet at 30bf0000 device
> TFTP from server 10.193.108.176; our IP address is 10.193.102.192; sending through gateway 10.193.102.254
> After:
> u-boot=> run netboot
> Booting from net ...
> BOOTP broadcast 1
> DHCP client bound to address 10.193.102.192 (454 ms)
> Using ethernet at 30bf0000 device
> TFTP from server 10.193.108.176; our IP address is 10.193.102.192; sending through gateway 10.193.102.254
>
> Signed-off-by: Joakim Zhang <qiangqing.zhang at nxp.com>
> ---
>   drivers/net/dwc_eth_qos.c | 132 +++++++++++++++++++-------------------
>   1 file changed, 67 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c
> index 585101804d..c1923fbe6b 100644
> --- a/drivers/net/dwc_eth_qos.c
> +++ b/drivers/net/dwc_eth_qos.c
> @@ -1045,20 +1045,6 @@ static int eqos_start(struct udevice *dev)
>   	eqos->tx_desc_idx = 0;
>   	eqos->rx_desc_idx = 0;
>   
> -	ret = eqos->config->ops->eqos_start_clks(dev);
> -	if (ret < 0) {
> -		pr_err("eqos_start_clks() failed: %d", ret);
> -		goto err;
> -	}
> -
> -	ret = eqos->config->ops->eqos_start_resets(dev);
> -	if (ret < 0) {
> -		pr_err("eqos_start_resets() failed: %d", ret);
> -		goto err_stop_clks;
> -	}
> -
> -	udelay(10);
> -
>   	eqos->reg_access_ok = true;

=> as clock is moved in probe...

the line
    eqos->reg_access_ok = true
should be also moved I think

or reg_access_ok can be removed, as reg_access_ok is always one when eqos_write_hwaddr is called

>   
>   	ret = wait_for_bit_le32(&eqos->dma_regs->mode,
> @@ -1066,68 +1052,34 @@ static int eqos_start(struct udevice *dev)
>   				eqos->config->swr_wait, false);
>   	if (ret) {
>   		pr_err("EQOS_DMA_MODE_SWR stuck");
> -		goto err_stop_resets;
> +		goto err;
>   	}
>   
>   	ret = eqos->config->ops->eqos_calibrate_pads(dev);
>   	if (ret < 0) {
>   		pr_err("eqos_calibrate_pads() failed: %d", ret);
> -		goto err_stop_resets;
> +		goto err;
>   	}
>   	rate = eqos->config->ops->eqos_get_tick_clk_rate(dev);
>   
>   	val = (rate / 1000000) - 1;
>   	writel(val, &eqos->mac_regs->us_tic_counter);
>   
> -	/*
> -	 * if PHY was already connected and configured,
> -	 * don't need to reconnect/reconfigure again
> -	 */
> -	if (!eqos->phy) {
> -		int addr = -1;
> -#ifdef CONFIG_DM_ETH_PHY
> -		addr = eth_phy_get_addr(dev);
> -#endif
> -#ifdef DWC_NET_PHYADDR
> -		addr = DWC_NET_PHYADDR;
> -#endif
> -		eqos->phy = phy_connect(eqos->mii, addr, dev,
> -					eqos->config->interface(dev));
> -		if (!eqos->phy) {
> -			pr_err("phy_connect() failed");
> -			goto err_stop_resets;
> -		}
> -
> -		if (eqos->max_speed) {
> -			ret = phy_set_supported(eqos->phy, eqos->max_speed);
> -			if (ret) {
> -				pr_err("phy_set_supported() failed: %d", ret);
> -				goto err_shutdown_phy;
> -			}
> -		}
> -
> -		ret = phy_config(eqos->phy);
> -		if (ret < 0) {
> -			pr_err("phy_config() failed: %d", ret);
> -			goto err_shutdown_phy;
> -		}
> -	}
> -
>   	ret = phy_startup(eqos->phy);
>   	if (ret < 0) {
>   		pr_err("phy_startup() failed: %d", ret);
> -		goto err_shutdown_phy;
> +		goto err;
>   	}
>   
>   	if (!eqos->phy->link) {
>   		pr_err("No link");
> -		goto err_shutdown_phy;
> +		goto err;
>   	}
>   
>   	ret = eqos_adjust_link(dev);
>   	if (ret < 0) {
>   		pr_err("eqos_adjust_link() failed: %d", ret);
> -		goto err_shutdown_phy;
> +		goto err;
>   	}
>   
>   	/* Configure MTL */
> @@ -1356,12 +1308,6 @@ static int eqos_start(struct udevice *dev)
>   	debug("%s: OK\n", __func__);
>   	return 0;
>   
> -err_shutdown_phy:
> -	phy_shutdown(eqos->phy);
> -err_stop_resets:
> -	eqos->config->ops->eqos_stop_resets(dev);
> -err_stop_clks:
> -	eqos->config->ops->eqos_stop_clks(dev);
>   err:
>   	pr_err("FAILED: %d", ret);
>   	return ret;
> @@ -1412,12 +1358,6 @@ static void eqos_stop(struct udevice *dev)
>   	clrbits_le32(&eqos->dma_regs->ch0_rx_control,
>   		     EQOS_DMA_CH0_RX_CONTROL_SR);
>   
> -	if (eqos->phy) {
> -		phy_shutdown(eqos->phy);
> -	}
> -	eqos->config->ops->eqos_stop_resets(dev);
> -	eqos->config->ops->eqos_stop_clks(dev);
> -
>   	debug("%s: OK\n", __func__);
>   }
>   
> @@ -1888,9 +1828,65 @@ static int eqos_probe(struct udevice *dev)
>   	eth_phy_set_mdio_bus(dev, eqos->mii);
>   #endif
>   
> +	ret = eqos->config->ops->eqos_start_clks(dev);
> +	if (ret < 0) {
> +		pr_err("eqos_start_clks() failed: %d", ret);
> +		goto err_unregister_mdio;
> +	}
> +
> +	ret = eqos->config->ops->eqos_start_resets(dev);
> +	if (ret < 0) {
> +		pr_err("eqos_start_resets() failed: %d", ret);
> +		goto err_stop_clks;
> +	}
> +
> +	udelay(10);
> +
> +	/*
> +	 * if PHY was already connected and configured,
> +	 * don't need to reconnect/reconfigure again
> +	 */
> +	if (!eqos->phy) {


this test can be remove I think

because we have always eqos->phy = NULL in probe,

PHY was can't be configured before probe

and it should be done again after a remove...


> +		int addr = -1;
> +#ifdef CONFIG_DM_ETH_PHY
> +		addr = eth_phy_get_addr(dev);
> +#endif
> +#ifdef DWC_NET_PHYADDR
> +		addr = DWC_NET_PHYADDR;
> +#endif
> +		eqos->phy = phy_connect(eqos->mii, addr, dev,
> +					eqos->config->interface(dev));
> +		if (!eqos->phy) {
> +			pr_err("phy_connect() failed");
> +			goto err_stop_resets;
> +		}
> +
> +		if (eqos->max_speed) {
> +			ret = phy_set_supported(eqos->phy, eqos->max_speed);
> +			if (ret) {
> +				pr_err("phy_set_supported() failed: %d", ret);
> +				goto err_shutdown_phy;
> +			}
> +		}
> +
> +		ret = phy_config(eqos->phy);
> +		if (ret < 0) {
> +			pr_err("phy_config() failed: %d", ret);
> +			goto err_shutdown_phy;
> +		}
> +	}
> +
>   	debug("%s: OK\n", __func__);
>   	return 0;
>   
> +err_shutdown_phy:
> +	phy_shutdown(eqos->phy);
> +err_stop_resets:
> +	eqos->config->ops->eqos_stop_resets(dev);
> +err_stop_clks:
> +	eqos->config->ops->eqos_stop_clks(dev);
> +err_unregister_mdio:
> +	mdio_unregister(eqos->mii);
>   err_free_mdio:
>   	mdio_free(eqos->mii);
>   err_remove_resources_tegra:
> @@ -1908,6 +1904,12 @@ static int eqos_remove(struct udevice *dev)
>   
>   	debug("%s(dev=%p):\n", __func__, dev);
>   
> +	if (eqos->phy) {
> +		phy_shutdown(eqos->phy);
> +	}
> +	eqos->config->ops->eqos_stop_resets(dev);
> +	eqos->config->ops->eqos_stop_clks(dev);
> +
>   	mdio_unregister(eqos->mii);
>   	mdio_free(eqos->mii);
>   	eqos->config->ops->eqos_remove_resources(dev);


Regards


Patrick



More information about the U-Boot mailing list