Hi Michal,
On 1/24/23 14:12, Michal Simek wrote:
Hi Stefan,
On 1/11/23 12:38, Stefan Roese wrote:
Hi Harini,
On 1/11/23 12:20, Katakam, Harini wrote:
+
Hi Stefan,
-----Original Message-----
From: U-Boot <u-boot-boun...@lists.denx.de> On Behalf Of Stefan Roese
Sent: Wednesday, January 11, 2023 1:01 PM
To: u-boot@lists.denx.de
Cc: Michal Simek <michal.si...@amd.com>; Ramon Fried
<rfried....@gmail.com>; Sean Anderson <sean.ander...@seco.com>
Subject: [PATCH] net: zynq_gem: Add a 10ms delay in zynq_gem_init()
In our system using ZynqMP with an external SGMII PHY it's necessary to
wait a short while after the configuration in zynq_gem_init() before
the xfer
starts. Otherwise the first packet(s) might get dropped, resulting
in a delay at
the start of the ethernet transfers.
This patch adds a minimal delay of 10ms which fixes problems of dropped
first packages.
Signed-off-by: Stefan Roese <s...@denx.de>
Cc: Michal Simek <michal.si...@amd.com>
Cc: Ramon Fried <rfried....@gmail.com>
Cc: Sean Anderson <sean.ander...@seco.com>
---
drivers/net/zynq_gem.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index
507b19b75975..26e468766871 100644
--- a/drivers/net/zynq_gem.c
+++ b/drivers/net/zynq_gem.c
@@ -522,6 +522,13 @@ static int zynq_gem_init(struct udevice *dev)
return ret;
}
}
+
+ /*
+ * Some additional minimal delay seems to be needed so that
+ * the first packet will be sent correctly
+ */
+ mdelay(10);
+
Thanks for the patch. The PCS status check in your v1 seems valid.
However, this delay of 10 msecs should not be necessary. If the PCS
status (when autoneg is enabled with the external PHY on your board)
shows link up, that's enough.
Unfortunately this is not the case. Regardless of using the PCS check
from the v1 patch from yesterday, such a minimal delay is necessary on
our setup.
Could you please consider the following
to investigate why initial packets are lost?
-> If the external PHY on your board has a hardware reset please
consider updating the reset-assert-us and reset-deassert-us properties
to ensure PHY is ready before access.
Not sure if this will help. See my comment at the end please for this.
-> Can you please check if there's any link stability issues in the
initial
msecs? Monitoring the serdes/GTR SGMII lane that's setup on ZynqMP
will be useful.
I tested 2 situations:
a) Starting network traffic very early in U-Boot, before the aneg link
establishment has finished (preboot=ping 192.168.1.5):
[8.448005 0.007878] Net:
[8.453711 0.005706] ZYNQ GEM: ff0d0000, mdio bus ff0d0000, phyaddr 0,
interface sgmii
[8.454449 0.000739] eth0: ethernet@ff0d0000
[8.454752 0.000303] ethernet@ff0d0000 Waiting for PHY auto negotiation
to complete..... done
[10.915882 2.461130] Using ethernet@ff0d0000 device
[10.916228 0.000346] host 192.168.1.5 is alive
[10.917247 0.001019] Hit any key to stop autoboot: 0
b) Late in U-Boot, after waiting for some seconds (or even minutes, it
makes no difference) on the prompt (preboot=sleep 10;ping 192.168.1.5):
[4.655932 0.007849] Net:
[4.661612 0.005680] ZYNQ GEM: ff0d0000, mdio bus ff0d0000, phyaddr 0,
interface sgmii
[4.662367 0.000755] eth0: ethernet@ff0d0000
[14.671655 10.009288] Using ethernet@ff0d0000 device
[14.672018 0.000364] host 192.168.1.5 is alive
[14.673032 0.001014] Hit any key to stop autoboot: 0
The delay is needed in both cases. In a) you see, that the PHY aneg
takes a bit of time to finish. The link is established at this time.
My feeling is that configuration of "pcscntrl" (or some other register)
after phy_startup() in zynq_gem_init() introduces this necessity of this
additional delay.
I think this will require further investigation what it is really
happening.
If this is related to SGMII only or also RGMII. Obviously adding some
delay without proper explanation is not the right way to go.
Agreed. I'm also not 100% satisfied with this "solution".
Pretty much if something is not stable there should be a way to pooling
any bit.
You mean polling (instead of pooling)? Again, the link is already
established when "pcscntrl" is configured. I can try to double-check
again, if I can poll for something in this "pcscntrl".
There is ongoing effort on moving clock from direct register access to
using firmware interface. And maybe this issue will be resolve by that
that simply clock is not stable at the time when driver tries to enable
RX/TX.
And current clock driver is not waiting for clock to be stable but
firmware base driver does it.
As mentioned above, my feeling is that configuration of "pcscntrl" (or
some other register) after phy_startup() in zynq_gem_init() introduces
this necessity of this additional delay. If this is the case, then
changes in the clock access won't make any difference here IIUTC.
Still I will do some more tests with this "pcscntrl" hopefully later
today and will report back. Stay tuned...
Thanks,
Stefan
