[U-Boot] [PATCH 1/3] FEC: Remove endless loop in the FEC driver
The FEC hardware sometimes errors out on data transfer and hangs in the tightloop adjusted by this patch. So add timeout into the tightloop to make such a hang recoverable.
Signed-off-by: Marek Vasut marex@denx.de Cc: Joe Hershberger joe.hershberger@ni.com Cc: Fabio Estevam festevam@gmail.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/net/fec_mxc.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index bc44d38..6f071e9 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -35,6 +35,12 @@
DECLARE_GLOBAL_DATA_PTR;
+/* + * Timeout the transfer after 5 mS. This is usually a bit more, since + * the code in the tightloops this timeout is used in adds some overhead. + */ +#define FEC_XFER_TIMEOUT 5000 + #ifndef CONFIG_MII #error "CONFIG_MII has to be defined!" #endif @@ -697,6 +703,8 @@ static int fec_send(struct eth_device *dev, void *packet, int length) unsigned int status; uint32_t size, end; uint32_t addr; + int timeout = FEC_XFER_TIMEOUT; + int ret = 0;
/* * This routine transmits one frame. This routine only accepts @@ -764,6 +772,10 @@ static int fec_send(struct eth_device *dev, void *packet, int length) while (readw(&fec->tbd_base[fec->tbd_index].status) & FEC_TBD_READY) { udelay(1); invalidate_dcache_range(addr, addr + size); + if (!timeout--) { + ret = -EINVAL; + break; + } }
debug("fec_send: status 0x%x index %d\n", @@ -775,7 +787,7 @@ static int fec_send(struct eth_device *dev, void *packet, int length) else fec->tbd_index = 1;
- return 0; + return ret; }
/**
The mechanism waiting for transmission to finish in fec_send() now relies on the E-bit being cleared in the TX buffer descriptor. In case of data cache being on, this means invalidation of data cache above this TX buffer descriptor on each test for the E-bit being cleared.
Apparently, there is another way to check if the transmission did complete. This is by checking the TDAR bit in the X_DES_ACTIVE register. Reading a register does not need any data cache invalidation, which is beneficial.
Rework the sequence that wait for completion of the transmission so that the TDAR bit is tested first and afterwards check the E-bit being clear. This cuts down the number of cache invalidation calls to one.
Signed-off-by: Marek Vasut marex@denx.de Cc: Joe Hershberger joe.hershberger@ni.com Cc: Fabio Estevam festevam@gmail.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/net/fec_mxc.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index 6f071e9..6ede464 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -768,19 +768,21 @@ static int fec_send(struct eth_device *dev, void *packet, int length) * invalidate data cache to see what's really in RAM. Also, we need * barrier here. */ - invalidate_dcache_range(addr, addr + size); - while (readw(&fec->tbd_base[fec->tbd_index].status) & FEC_TBD_READY) { - udelay(1); - invalidate_dcache_range(addr, addr + size); - if (!timeout--) { - ret = -EINVAL; + while (--timeout) { + if (!(readl(&fec->eth->x_des_active) & (1 << 24))) break; - } }
- debug("fec_send: status 0x%x index %d\n", + if (!timeout) + ret = -EINVAL; + + invalidate_dcache_range(addr, addr + size); + if (readw(&fec->tbd_base[fec->tbd_index].status) & FEC_TBD_READY) + ret = -EINVAL; + + debug("fec_send: status 0x%x index %d ret %i\n", readw(&fec->tbd_base[fec->tbd_index].status), - fec->tbd_index); + fec->tbd_index, ret); /* for next transmission use the other buffer */ if (fec->tbd_index) fec->tbd_index = 0;
Hi Marek,
On Wed, Aug 29, 2012 at 8:49 AM, Marek Vasut marex@denx.de wrote:
The mechanism waiting for transmission to finish in fec_send() now relies on the E-bit being cleared in the TX buffer descriptor. In case of data cache being on, this means invalidation of data cache above this TX buffer descriptor on each test for the E-bit being cleared.
Apparently, there is another way to check if the transmission did complete. This is by checking the TDAR bit in the X_DES_ACTIVE register. Reading a register does not need any data cache invalidation, which is beneficial.
Rework the sequence that wait for completion of the transmission so that the TDAR bit is tested first and afterwards check the E-bit being clear. This cuts down the number of cache invalidation calls to one.
Signed-off-by: Marek Vasut marex@denx.de Cc: Joe Hershberger joe.hershberger@ni.com Cc: Fabio Estevam festevam@gmail.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de
Applied, thanks.
-Joe
Replace the magic contant 1 << 24 with properly defined bits.
Signed-off-by: Marek Vasut marex@denx.de Cc: Joe Hershberger joe.hershberger@ni.com Cc: Fabio Estevam festevam@gmail.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de --- drivers/net/fec_mxc.c | 6 +++--- drivers/net/fec_mxc.h | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index 6ede464..3e232c7 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -256,7 +256,7 @@ static int miiphy_wait_aneg(struct eth_device *dev)
static int fec_rx_task_enable(struct fec_priv *fec) { - writel(1 << 24, &fec->eth->r_des_active); + writel(FEC_R_DES_ACTIVE_RDAR, &fec->eth->r_des_active); return 0; }
@@ -267,7 +267,7 @@ static int fec_rx_task_disable(struct fec_priv *fec)
static int fec_tx_task_enable(struct fec_priv *fec) { - writel(1 << 24, &fec->eth->x_des_active); + writel(FEC_X_DES_ACTIVE_TDAR, &fec->eth->x_des_active); return 0; }
@@ -769,7 +769,7 @@ static int fec_send(struct eth_device *dev, void *packet, int length) * barrier here. */ while (--timeout) { - if (!(readl(&fec->eth->x_des_active) & (1 << 24))) + if (!(readl(&fec->eth->x_des_active) & FEC_X_DES_ACTIVE_TDAR)) break; }
diff --git a/drivers/net/fec_mxc.h b/drivers/net/fec_mxc.h index 852b2e0..203285a 100644 --- a/drivers/net/fec_mxc.h +++ b/drivers/net/fec_mxc.h @@ -213,6 +213,9 @@ struct ethernet_regs {
#define FEC_X_WMRK_STRFWD 0x00000100
+#define FEC_X_DES_ACTIVE_TDAR 0x01000000 +#define FEC_R_DES_ACTIVE_RDAR 0x01000000 + #if defined(CONFIG_MX25) || defined(CONFIG_MX53) /* defines for MIIGSK */ /* RMII frequency control: 0=50MHz, 1=5MHz */
Hi Marek,
On Wed, Aug 29, 2012 at 8:49 AM, Marek Vasut marex@denx.de wrote:
Replace the magic contant 1 << 24 with properly defined bits.
Signed-off-by: Marek Vasut marex@denx.de Cc: Joe Hershberger joe.hershberger@ni.com Cc: Fabio Estevam festevam@gmail.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de
Applied, thanks.
-Joe
Hi Marek,
On Wed, Aug 29, 2012 at 8:49 AM, Marek Vasut marex@denx.de wrote:
The FEC hardware sometimes errors out on data transfer and hangs in the tightloop adjusted by this patch. So add timeout into the tightloop to make such a hang recoverable.
Signed-off-by: Marek Vasut marex@denx.de Cc: Joe Hershberger joe.hershberger@ni.com Cc: Fabio Estevam festevam@gmail.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de
Applied, thanks.
-Joe
Dear Joe Hershberger,
Hi Marek,
On Wed, Aug 29, 2012 at 8:49 AM, Marek Vasut marex@denx.de wrote:
The FEC hardware sometimes errors out on data transfer and hangs in the tightloop adjusted by this patch. So add timeout into the tightloop to make such a hang recoverable.
Signed-off-by: Marek Vasut marex@denx.de Cc: Joe Hershberger joe.hershberger@ni.com Cc: Fabio Estevam festevam@gmail.com Cc: Otavio Salvador otavio@ossystems.com.br Cc: Stefano Babic sbabic@denx.de
Applied, thanks.
Doesn't this go via -imx ? Anyway, all these FEC patches from me MUST go into current release. That is imperative, they are important bugfixes.
-Joe
Best regards, Marek Vasut
participants (2)
-
Joe Hershberger -
Marek Vasut