[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