[U-Boot] FEC and EFI Simple Network

* [U-Boot] FEC and EFI Simple Network
@ 2018-03-27 12:12 Patrick Wildt
  2018-03-28 21:54 ` Fabio Estevam
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick Wildt @ 2018-03-27 12:12 UTC (permalink / raw)
  To: u-boot

Hi,

I have been debugging network issues when running an EFI Application
that uses the EFI Simple Network protocol on an i.MX6 machine (FEC).

The symptom is that u-boot's (FEC) internal RX ring index is reset to 0,
while the controller is still at idx 3 (or something else).  This is
caused by the following circumstances:

The Simple Network protocol offers methods like Start(), Stop(),
Initialize(), Shutdown(), Reset().  Also the protocol has a state.  The
states are Stopped, Started, Initialized.  The transition is as follows:

Stopped ---Start()---> Started ---Initialize()--> Initialized

Start() does some initialization, Initialize() allocates the TX/RX
descriptors and actually kicks off the network engine.

So far, only Initialize() is implemented in our u-boot interface, and it
calls eth_init() which in the end calls fec_init().  Our network state
is _always_ Started.  This means that EFI Applications see that that the
state is Started and then call Initialize() to start the actual network
traffic.  There is no call to Stop()/Shutdown()/Reset() as it's supposed
to be in a sane state.

In my case the FEC is already initialized since I booted using network
and the RX desc index is already non-zero.  Now the EFI Application sees
that the state is Started, calls Initialize() which makes u-boot call
eth_init() which then calls fec_init().

fec_init() does not reset the controller so that the controller-internal
RX desc index is not reset to zero.  fec_init() calls fec_open() which
then resets the driver-internal RX desc index to zero.  Now they are out
of sync, boom.

This means that fec_init() without a previous fec_halt() breaks the
whole network if it was already running.  The Designware driver as used
by some sunxi platforms does a reset of the controller in the init
function.  Maybe calling fec_halt() at the start of fec_init() could be
a possible solution?

Patrick

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index ff7ad91116..ba8bd9920d 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -522,6 +522,12 @@ static int fec_open(struct eth_device *edev)
 	return 0;
 }
 
+#ifdef CONFIG_DM_ETH
+static void fecmxc_halt(struct udevice *dev);
+#else
+static void fec_halt(struct eth_device *dev);
+#endif
+
 #ifdef CONFIG_DM_ETH
 static int fecmxc_init(struct udevice *dev)
 #else
@@ -537,6 +543,15 @@ static int fec_init(struct eth_device *dev, bd_t *bd)
 	u8 *i;
 	ulong addr;
 
+#ifdef CONFIG_DM_ETH
+	fecmxc_halt(dev);
+#else
+	fec_halt(dev);
+#endif
+
+	writel(~FEC_TCNTRL_GTS & readl(&fec->eth->x_cntrl),
+	       &fec->eth->x_cntrl);
+
 	/* Initialize MAC address */
 #ifdef CONFIG_DM_ETH
 	fecmxc_set_hwaddr(dev);
@@ -825,19 +840,12 @@ static int fec_recv(struct eth_device *dev)
 	}
 	if (ievent & FEC_IEVENT_HBERR) {
 		/* Heartbeat error */
-		writel(0x00000001 | readl(&fec->eth->x_cntrl),
+		writel(FEC_TCNTRL_GTS | readl(&fec->eth->x_cntrl),
 		       &fec->eth->x_cntrl);
 	}
 	if (ievent & FEC_IEVENT_GRA) {
 		/* Graceful stop complete */
-		if (readl(&fec->eth->x_cntrl) & 0x00000001) {
-#ifdef CONFIG_DM_ETH
-			fecmxc_halt(dev);
-#else
-			fec_halt(dev);
-#endif
-			writel(~0x00000001 & readl(&fec->eth->x_cntrl),
-			       &fec->eth->x_cntrl);
+		if (readl(&fec->eth->x_cntrl) & FEC_TCNTRL_GTS) {
 #ifdef CONFIG_DM_ETH
 			fecmxc_init(dev);
 #else

^ permalink raw reply related	[flat|nested] 6+ messages in thread