RE: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
Hi Ramon, > -Original Message- > From: Ramon Fried > Sent: 2021年11月17日 12:20 > To: Joakim Zhang > Cc: Joe Hershberger ; U-Boot Mailing List > ; Ye Li ; Patrick Delaunay > ; Daniil Stas ; > Stephen Warren > 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 > wrote: > > > > > > > -Original Message- > > > From: Ramon Fried > > > Sent: 2021年11月16日 18:45 > > > To: Joakim Zhang > > > Cc: Joe Hershberger ; U-Boot Mailing List > > > ; Ye Li ; Patrick Delaunay > > > ; Daniil Stas > > > ; Stephen Warren > > > 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 > > > > > > wrote: > > > > > > > > > > > > Hi Ramon, > > > > > > > > > -Original Message- > > > > > From: Ramon Fried > > > > > Sent: 2021年11月16日 13:57 > > > > > To: Joakim Zhang > > > > > Cc: Joe Hershberger ; U-Boot Mailing > > > > > List ; Ye Li ; Patrick > > > > > Delaunay ; Daniil Stas > > > > > ; Stephen Warren > > > > > 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 > > > > > > > > > > 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@30bf Waiting for PHY auto negotiation to > complete... > > > > > > done BOOTP broadcast 1 DHCP client bound to address > > > > > > 10.193.102.192 > > > > > > (313 ms) Using ethernet@30bf 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@30bf 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@30bf Waiting for PHY
RE: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
> -Original Message- > From: Ramon Fried > Sent: 2021年11月16日 18:45 > To: Joakim Zhang > Cc: Joe Hershberger ; U-Boot Mailing List > ; Ye Li ; Patrick Delaunay > ; Daniil Stas ; > Stephen Warren > 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 > wrote: > > > > > > Hi Ramon, > > > > > -Original Message- > > > From: Ramon Fried > > > Sent: 2021年11月16日 13:57 > > > To: Joakim Zhang > > > Cc: Joe Hershberger ; U-Boot Mailing List > > > ; Ye Li ; Patrick Delaunay > > > ; Daniil Stas > > > ; Stephen Warren > > > 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 > > > > > > 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@30bf Waiting for PHY auto negotiation to complete... > > > > done BOOTP broadcast 1 DHCP client bound to address 10.193.102.192 > > > > (313 ms) Using ethernet@30bf 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@30bf 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@30bf 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://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;
RE: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
Hi Ramon, > -Original Message- > From: Ramon Fried > Sent: 2021年11月16日 13:57 > To: Joakim Zhang > Cc: Joe Hershberger ; U-Boot Mailing List > ; Ye Li ; Patrick Delaunay > ; Daniil Stas ; > Stephen Warren > 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 > 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@30bf Waiting for PHY auto negotiation to complete... > > done BOOTP broadcast 1 DHCP client bound to address 10.193.102.192 > > (313 ms) Using ethernet@30bf 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@30bf 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@30bf 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#L225 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
RE: [PATCH] net: eqos: connect and config PHY from probe stage instead of starting EQOS
Hi Patrick, > -Original Message- > From: Patrick DELAUNAY > Sent: 2021年11月15日 18:42 > To: Joakim Zhang ; joe.hershber...@ni.com; > rfried@gmail.com > Cc: u-boot@lists.denx.de; Ye Li ; daniil.s...@posteo.net; > swar...@nvidia.com; Christophe ROULLIER > ; Marek Vasut > 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@30bf Waiting for PHY auto negotiation to complete... > > done BOOTP broadcast 1 DHCP client bound to address 10.193.102.192 > > (313 ms) Using ethernet@30bf 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@30bf 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 > > --- > > 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 / 100) - 1; > > writel(val, &eqos->mac_regs->us_tic_counter); > > > > - /* > > -* if PHY was already connected and configured, > > -* don'
[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@30bf Waiting for PHY auto negotiation to complete... done BOOTP broadcast 1 DHCP client bound to address 10.193.102.192 (313 ms) Using ethernet@30bf 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@30bf 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 --- 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 / 100) - 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