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
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%23L225data=04%7C01%7Cqiangqing.zhang%40nxp.com%
> 7C59322db
> >
> 193a54788a09308d9a8ee2cfc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%
> 7C0%7
> >
> C637726563178464522%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIj
> >
> oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=usRIR7L
> exBnk
> > dnDO3U8hHYtMAIWT8bJ0Q7wgTElUjHs%3Dreserved=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(>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, >mac_regs->us_tic_counter);
> >
> > - /*
> > -* if PHY was already connected and configured,
> > -* don't need to reconnec
[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(>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, >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
