Hi All For information
Another patch from Marek will collide with this one => https://patchwork.ozlabs.org/project/uboot/patch/20211113022352.231762-1-ma...@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@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@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@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.zh...@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 >
