[PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
Patrice CHOTARD
patrice.chotard at foss.st.com
Mon Nov 15 11:46:34 CET 2021
Hi All
For information
Another patch from Marek will collide with this one => https://patchwork.ozlabs.org/project/uboot/patch/20211113022352.231762-1-marex@denx.de/
Patrice
On 11/15/21 11:41 AM, Patrick DELAUNAY wrote:
> 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