[PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
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 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.zhang@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;
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) { + 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);
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.zhang@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
Hi All
For information
Another patch from Marek will collide with this one => https://patchwork.ozlabs.org/project/uboot/patch/20211113022352.231762-1-mar...
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.zhang@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
Hi Patrick,
-----Original Message----- From: Patrick DELAUNAY patrick.delaunay@foss.st.com Sent: 2021年11月15日 18:42 To: Joakim Zhang qiangqing.zhang@nxp.com; joe.hershberger@ni.com; rfried.dev@gmail.com Cc: u-boot@lists.denx.de; Ye Li ye.li@nxp.com; daniil.stas@posteo.net; swarren@nvidia.com; Christophe ROULLIER christophe.roullier@foss.st.com; Marek Vasut marex@denx.de Subject: Re: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
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.zhang@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
Why? I saw this variable "reg_access_ok " doesn't have initialize value. If remove this line, I think it will break the logic in eqos_write_hwaddr(), so I agree also move it into probe().
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...
Agree, I will remove this check.
Best Regards, Joakim Zhang
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
On Wed, Nov 10, 2021 at 7:42 AM Joakim Zhang qiangqing.zhang@nxp.com 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 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
How much time do you save exactly, Is it the ~140ms you show here at the commit log ?
Signed-off-by: Joakim Zhang qiangqing.zhang@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; 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) {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);-- 2.17.1
Hi Ramon,
-----Original Message----- From: Ramon Fried rfried.dev@gmail.com Sent: 2021年11月16日 13:57 To: Joakim Zhang qiangqing.zhang@nxp.com Cc: Joe Hershberger joe.hershberger@ni.com; U-Boot Mailing List u-boot@lists.denx.de; Ye Li ye.li@nxp.com; Patrick Delaunay patrick.delaunay@foss.st.com; Daniil Stas daniil.stas@posteo.net; Stephen Warren swarren@nvidia.com Subject: Re: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
On Wed, Nov 10, 2021 at 7:42 AM Joakim Zhang qiangqing.zhang@nxp.com 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 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
How much time do you save exactly, Is it the ~140ms you show here at the commit log ?
Exactly not. This time points to DHCP client bound to address, not the time this patch saves.
How can we evaluate the time we save? Please see the log:
Before: Booting from net ... ethernet@30bf0000 Waiting for PHY auto negotiation to complete....... done BOOTP broadcast 1 After: Booting from net ... BOOTP broadcast 1
And you need to check the related code: drivers/net/dwc_eth_qos.c: phy_startup ->...-> drivers/net/phy/phy.c: genphy_update_link()
https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/net/phy/phy.c#L22...
while (!(mii_reg & BMSR_ANEGCOMPLETE)) { /* * Timeout reached ? */ if (i > (PHY_ANEG_TIMEOUT / 50)) { printf(" TIMEOUT !\n"); phydev->link = 0; return -ETIMEDOUT; }
if (ctrlc()) { puts("user interrupt!\n"); phydev->link = 0; return -EINTR; }
if ((i++ % 10) == 0) printf(".");
mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMSR); mdelay(50); /* 50 ms */ }
We can see that one "." is about 500ms, so from the log, we save about 500*7=3500ms=3.5s, this also means that PHY needs about 3.5s to complete auto-nego.
I also described this in the commit message, "users need wait seconds for PHY auto negotiation to complete when do tftp boot", may not quite clear, I will improve it.
Best Regards, Joakim Zhang
On Tue, Nov 16, 2021 at 10:04 AM Joakim Zhang qiangqing.zhang@nxp.com wrote:
Hi Ramon,
-----Original Message----- From: Ramon Fried rfried.dev@gmail.com Sent: 2021年11月16日 13:57 To: Joakim Zhang qiangqing.zhang@nxp.com Cc: Joe Hershberger joe.hershberger@ni.com; U-Boot Mailing List u-boot@lists.denx.de; Ye Li ye.li@nxp.com; Patrick Delaunay patrick.delaunay@foss.st.com; Daniil Stas daniil.stas@posteo.net; Stephen Warren swarren@nvidia.com Subject: Re: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
On Wed, Nov 10, 2021 at 7:42 AM Joakim Zhang qiangqing.zhang@nxp.com 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 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
How much time do you save exactly, Is it the ~140ms you show here at the commit log ?
Exactly not. This time points to DHCP client bound to address, not the time this patch saves.
How can we evaluate the time we save? Please see the log:
Before: Booting from net ... ethernet@30bf0000 Waiting for PHY auto negotiation to complete....... done BOOTP broadcast 1 After: Booting from net ... BOOTP broadcast 1
And you need to check the related code: drivers/net/dwc_eth_qos.c: phy_startup ->...-> drivers/net/phy/phy.c: genphy_update_link()
https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/net/phy/phy.c#L22...
while (!(mii_reg & BMSR_ANEGCOMPLETE)) { /* * Timeout reached ? */ if (i > (PHY_ANEG_TIMEOUT / 50)) { printf(" TIMEOUT !\n"); phydev->link = 0; return -ETIMEDOUT; } if (ctrlc()) { puts("user interrupt!\n"); phydev->link = 0; return -EINTR; } if ((i++ % 10) == 0) printf("."); mii_reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMSR); mdelay(50); /* 50 ms */ }We can see that one "." is about 500ms, so from the log, we save about 500*7=3500ms=3.5s, this also means that PHY needs about 3.5s to complete auto-nego.
I also described this in the commit message, "users need wait seconds for PHY auto negotiation to complete when do tftp boot", may not quite clear, I will improve it.
Best Regards, Joakim Zhang
NACK. Hi Joakim. It's currently working like that for all network drivers, it's not specific What you're currently suggesting is that the link will be brought up automatically for all users, even if they're not interested in network booting. That won't work. Thanks, Ramon
-----Original Message----- From: Ramon Fried rfried.dev@gmail.com Sent: 2021年11月16日 18:45 To: Joakim Zhang qiangqing.zhang@nxp.com Cc: Joe Hershberger joe.hershberger@ni.com; U-Boot Mailing List u-boot@lists.denx.de; Ye Li ye.li@nxp.com; Patrick Delaunay patrick.delaunay@foss.st.com; Daniil Stas daniil.stas@posteo.net; Stephen Warren swarren@nvidia.com Subject: Re: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
On Tue, Nov 16, 2021 at 10:04 AM Joakim Zhang qiangqing.zhang@nxp.com wrote:
Hi Ramon,
-----Original Message----- From: Ramon Fried rfried.dev@gmail.com Sent: 2021年11月16日 13:57 To: Joakim Zhang qiangqing.zhang@nxp.com Cc: Joe Hershberger joe.hershberger@ni.com; U-Boot Mailing List u-boot@lists.denx.de; Ye Li ye.li@nxp.com; Patrick Delaunay patrick.delaunay@foss.st.com; Daniil Stas daniil.stas@posteo.net; Stephen Warren swarren@nvidia.com Subject: Re: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
On Wed, Nov 10, 2021 at 7:42 AM Joakim Zhang qiangqing.zhang@nxp.com 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 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
How much time do you save exactly, Is it the ~140ms you show here at the commit log ?
Exactly not. This time points to DHCP client bound to address, not the time
this patch saves.
How can we evaluate the time we save? Please see the log:
Before: Booting from net ... ethernet@30bf0000 Waiting for PHY auto negotiation to complete.......
done
BOOTP broadcast 1After: Booting from net ... BOOTP broadcast 1
And you need to check the related code: drivers/net/dwc_eth_qos.c: phy_startup ->...-> drivers/net/phy/phy.c: genphy_update_link()
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
ce.denx.de%2Fu-boot%2Fu-boot%2F-%2Fblob%2Fmaster%2Fdrivers%2Fnet %2Fphy
%2Fphy.c%23L225&data=04%7C01%7Cqiangqing.zhang%40nxp.com%
7C59322db
193a54788a09308d9a8ee2cfc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0% 7C0%7
C637726563178464522%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw MDAiLCJQIj
oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=usRIR7L exBnk
dnDO3U8hHYtMAIWT8bJ0Q7wgTElUjHs%3D&reserved=0
while (!(mii_reg & BMSR_ANEGCOMPLETE)) { /* * Timeout reached ? */ if (i > (PHY_ANEG_TIMEOUT / 50)) { printf(" TIMEOUT !\n"); phydev->link = 0; return -ETIMEDOUT; } if (ctrlc()) { puts("user interrupt!\n"); phydev->link = 0; return -EINTR; } if ((i++ % 10) == 0) printf("."); mii_reg = phy_read(phydev,MDIO_DEVAD_NONE, MII_BMSR);
mdelay(50); /* 50 ms */ }We can see that one "." is about 500ms, so from the log, we save about
500*7=3500ms=3.5s, this also means that PHY needs about 3.5s to complete auto-nego.
I also described this in the commit message, "users need wait seconds for
PHY auto negotiation to complete when do tftp boot", may not quite clear, I will improve it.
Best Regards, Joakim Zhang
NACK. Hi Joakim. It's currently working like that for all network drivers, it's not specific What you're currently suggesting is that the link will be brought up automatically for all users, even if they're not interested in network booting. That won't work.
Hi Ramon,
I am not quite understand you mean, I only connect and config the PHY in the MAC probe stage, yes, the link should be brought up, but startup PHY still in MAC open operation, just attaching PHY to MAC in MAC probe, seems not a problem. Is there any side effect?
We did the same thing in FEC driver: https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/net/fec_mxc.c#L15...
Best Regards, Joakim Zhang
Thanks, Ramon
On Tue, Nov 16, 2021 at 1:02 PM Joakim Zhang qiangqing.zhang@nxp.com wrote:
-----Original Message----- From: Ramon Fried rfried.dev@gmail.com Sent: 2021年11月16日 18:45 To: Joakim Zhang qiangqing.zhang@nxp.com Cc: Joe Hershberger joe.hershberger@ni.com; U-Boot Mailing List u-boot@lists.denx.de; Ye Li ye.li@nxp.com; Patrick Delaunay patrick.delaunay@foss.st.com; Daniil Stas daniil.stas@posteo.net; Stephen Warren swarren@nvidia.com Subject: Re: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
On Tue, Nov 16, 2021 at 10:04 AM Joakim Zhang qiangqing.zhang@nxp.com wrote:
Hi Ramon,
-----Original Message----- From: Ramon Fried rfried.dev@gmail.com Sent: 2021年11月16日 13:57 To: Joakim Zhang qiangqing.zhang@nxp.com Cc: Joe Hershberger joe.hershberger@ni.com; U-Boot Mailing List u-boot@lists.denx.de; Ye Li ye.li@nxp.com; Patrick Delaunay patrick.delaunay@foss.st.com; Daniil Stas daniil.stas@posteo.net; Stephen Warren swarren@nvidia.com Subject: Re: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
On Wed, Nov 10, 2021 at 7:42 AM Joakim Zhang qiangqing.zhang@nxp.com 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 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
How much time do you save exactly, Is it the ~140ms you show here at the commit log ?
Exactly not. This time points to DHCP client bound to address, not the time
this patch saves.
How can we evaluate the time we save? Please see the log:
Before: Booting from net ... ethernet@30bf0000 Waiting for PHY auto negotiation to complete.......
done
BOOTP broadcast 1After: Booting from net ... BOOTP broadcast 1
And you need to check the related code: drivers/net/dwc_eth_qos.c: phy_startup ->...-> drivers/net/phy/phy.c: genphy_update_link()
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
ce.denx.de%2Fu-boot%2Fu-boot%2F-%2Fblob%2Fmaster%2Fdrivers%2Fnet %2Fphy
%2Fphy.c%23L225&data=04%7C01%7Cqiangqing.zhang%40nxp.com%
7C59322db
193a54788a09308d9a8ee2cfc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0% 7C0%7
C637726563178464522%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw MDAiLCJQIj
oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=usRIR7L exBnk
dnDO3U8hHYtMAIWT8bJ0Q7wgTElUjHs%3D&reserved=0
while (!(mii_reg & BMSR_ANEGCOMPLETE)) { /* * Timeout reached ? */ if (i > (PHY_ANEG_TIMEOUT / 50)) { printf(" TIMEOUT !\n"); phydev->link = 0; return -ETIMEDOUT; } if (ctrlc()) { puts("user interrupt!\n"); phydev->link = 0; return -EINTR; } if ((i++ % 10) == 0) printf("."); mii_reg = phy_read(phydev,MDIO_DEVAD_NONE, MII_BMSR);
mdelay(50); /* 50 ms */ }We can see that one "." is about 500ms, so from the log, we save about
500*7=3500ms=3.5s, this also means that PHY needs about 3.5s to complete auto-nego.
I also described this in the commit message, "users need wait seconds for
PHY auto negotiation to complete when do tftp boot", may not quite clear, I will improve it.
Best Regards, Joakim Zhang
NACK. Hi Joakim. It's currently working like that for all network drivers, it's not specific What you're currently suggesting is that the link will be brought up automatically for all users, even if they're not interested in network booting. That won't work.
Hi Ramon,
I am not quite understand you mean, I only connect and config the PHY in the MAC probe stage, yes, the link should be brought up, but startup PHY still in MAC open operation, just attaching PHY to MAC in MAC probe, seems not a problem. Is there any side effect?
The link up is a side effect, the U-Boot device model works in lazy loading mode, where stuff is initialized only when needed. This works the same for all phys in all places.
Additionally, I'm not following how much time can you actually save, how much time passes between the probe() function and start() function in your setup ? how could it be seconds ?
We did the same thing in FEC driver: https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/net/fec_mxc.c#L15...
True, that's an example that contradicts what I just said, but it's wrong.
Best Regards, Joakim Zhang
Thanks, Ramon
Hi Ramon,
-----Original Message----- From: Ramon Fried rfried.dev@gmail.com Sent: 2021年11月17日 12:20 To: Joakim Zhang qiangqing.zhang@nxp.com Cc: Joe Hershberger joe.hershberger@ni.com; U-Boot Mailing List u-boot@lists.denx.de; Ye Li ye.li@nxp.com; Patrick Delaunay patrick.delaunay@foss.st.com; Daniil Stas daniil.stas@posteo.net; Stephen Warren swarren@nvidia.com Subject: Re: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
On Tue, Nov 16, 2021 at 1:02 PM Joakim Zhang qiangqing.zhang@nxp.com wrote:
-----Original Message----- From: Ramon Fried rfried.dev@gmail.com Sent: 2021年11月16日 18:45 To: Joakim Zhang qiangqing.zhang@nxp.com Cc: Joe Hershberger joe.hershberger@ni.com; U-Boot Mailing List u-boot@lists.denx.de; Ye Li ye.li@nxp.com; Patrick Delaunay patrick.delaunay@foss.st.com; Daniil Stas daniil.stas@posteo.net; Stephen Warren swarren@nvidia.com Subject: Re: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
On Tue, Nov 16, 2021 at 10:04 AM Joakim Zhang qiangqing.zhang@nxp.com wrote:
Hi Ramon,
-----Original Message----- From: Ramon Fried rfried.dev@gmail.com Sent: 2021年11月16日 13:57 To: Joakim Zhang qiangqing.zhang@nxp.com Cc: Joe Hershberger joe.hershberger@ni.com; U-Boot Mailing List u-boot@lists.denx.de; Ye Li ye.li@nxp.com; Patrick Delaunay patrick.delaunay@foss.st.com; Daniil Stas daniil.stas@posteo.net; Stephen Warren swarren@nvidia.com Subject: Re: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
On Wed, Nov 10, 2021 at 7:42 AM Joakim Zhang qiangqing.zhang@nxp.com 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 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
How much time do you save exactly, Is it the ~140ms you show here at the commit log ?
Exactly not. This time points to DHCP client bound to address, not the time
this patch saves.
How can we evaluate the time we save? Please see the log:
Before: Booting from net ... ethernet@30bf0000 Waiting for PHY auto negotiation to
complete.......
done
BOOTP broadcast 1After: Booting from net ... BOOTP broadcast 1
And you need to check the related code: drivers/net/dwc_eth_qos.c: phy_startup ->...-> drivers/net/phy/phy.c: genphy_update_link()
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fso
ur
ce.denx.de%2Fu-boot%2Fu-boot%2F-%2Fblob%2Fmaster%2Fdrivers%2Fnet
%2Fphy
%2Fphy.c%23L225&data=04%7C01%7Cqiangqing.zhang%40nxp.co
m%
7C59322db
193a54788a09308d9a8ee2cfc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%
7C0%7
C637726563178464522%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
MDAiLCJQIj
oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=usRIR7L
exBnk
dnDO3U8hHYtMAIWT8bJ0Q7wgTElUjHs%3D&reserved=0
while (!(mii_reg & BMSR_ANEGCOMPLETE)) { /* * Timeout reached ? */ if (i > (PHY_ANEG_TIMEOUT / 50)) { printf(" TIMEOUT !\n"); phydev->link = 0; return -ETIMEDOUT; } if (ctrlc()) { puts("user interrupt!\n"); phydev->link = 0; return -EINTR; } if ((i++ % 10) == 0) printf("."); mii_reg = phy_read(phydev,MDIO_DEVAD_NONE, MII_BMSR);
mdelay(50); /* 50 ms */ }We can see that one "." is about 500ms, so from the log, we save about
500*7=3500ms=3.5s, this also means that PHY needs about 3.5s to complete auto-nego.
I also described this in the commit message, "users need wait seconds for
PHY auto negotiation to complete when do tftp boot", may not quite clear, I will improve it.
Best Regards, Joakim Zhang
NACK. Hi Joakim. It's currently working like that for all network drivers, it's not specific What you're currently suggesting is that the link will be brought up automatically for all users, even if they're not interested in
network booting.
That won't work.
Hi Ramon,
I am not quite understand you mean, I only connect and config the PHY in the MAC probe stage, yes, the link should be brought up, but startup
PHY still in MAC open operation, just attaching PHY to MAC in MAC probe, seems not a problem. Is there any side effect? The link up is a side effect, the U-Boot device model works in lazy loading mode, where stuff is initialized only when needed. This works the same for all phys in all places.
Understand, thanks.
Additionally, I'm not following how much time can you actually save, how much time passes between the probe() function and start() function in your setup ? how could it be seconds ?
Actually, Realtek PHY(RTL8211FDI) at my side, it needs about 3.5s to complete auto-negotiation. If we connect and config PHY in probe(), it _may_ complete auto-nego when user start network (start()), as you said, yes, if we start network quickly, we also need some time to wait it to complete. However, at least, we can save some time compared to connect and config PHY in start(), where we need wait about 3.5s to complete auto-nego.
We did the same thing in FEC driver:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
ce.denx.de%2Fu-boot%2Fu-boot%2F-%2Fblob%2Fmaster%2Fdrivers%2Fnet %2Ffec
_mxc.c%23L1522&data=04%7C01%7Cqiangqing.zhang%40nxp.com%7C4 3be042d
7b6f427f406708d9a98188f8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7 C0%7C
637727196088131120%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM DAiLCJQIjo
iV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=bElnoO13 uq0kg
t7yzIFwiMJpVH3L49SNF5dssplt%2Fjw%3D&reserved=0
True, that's an example that contradicts what I just said, but it's wrong.
OK.
Best Regards, Joakim Zhang
Best Regards, Joakim Zhang
Thanks, Ramon
participants (4)
-
Joakim Zhang -
Patrice CHOTARD -
Patrick DELAUNAY -
Ramon Fried