Patchwork [U-Boot] FEC: Rework RX buffer init

login
register
mail settings
Submitter Marek Vasut
Date Oct. 9, 2012, 10:43 p.m.
Message ID <1349822613-26137-1-git-send-email-marex@denx.de>
Download mbox | patch
Permalink /patch/190503/
State Superseded
Delegated to: Stefano Babic
Headers show

Comments

Marek Vasut - Oct. 9, 2012, 10:43 p.m.
Rework the RX buffer init so that it's easier to understand.
Firstly, allocate the whole RX buffer as one large continuous
piece of memory. Also, drop all these writel() accessors used
on the FEC RX DMA descriptor, since it's all flat bogus.

Finally, this makes recoverable stalls of the FEC on i.MX28
disappear completely.

Also, it removes whole 80 bytes from the size of u-boot!
(Tested with Debian GCC 4.7.1-3)

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Joe Hershberger <joe.hershberger@ni.com>
Cc: Otavio Salvador <otavio@ossystems.com.br>
Cc: Stefano Babic <sbabic@denx.de>
---
 drivers/net/fec_mxc.c |   47 ++++++++++++++++++++---------------------------
 1 file changed, 20 insertions(+), 27 deletions(-)
Albert ARIBAUD - Oct. 12, 2012, 11:08 a.m.
Hi Marek,

On Wed, 10 Oct 2012 00:43:33 +0200, Marek Vasut <marex@denx.de> wrote:

> Rework the RX buffer init so that it's easier to understand.
> Firstly, allocate the whole RX buffer as one large continuous
> piece of memory. Also, drop all these writel() accessors used
> on the FEC RX DMA descriptor, since it's all flat bogus.

I'd like this commit message to be more usefully descriptive than "all
flat bogus". What exactly is bogus in it?

> Finally, this makes recoverable stalls of the FEC on i.MX28
> disappear completely.

Are both the single allocation change and the writel() change needed
for this?

Amicalement,

Patch

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 3e232c7..1897397 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -289,43 +289,36 @@  static int fec_tx_task_disable(struct fec_priv *fec)
  */
 static int fec_rbd_init(struct fec_priv *fec, int count, int dsize)
 {
+	struct fec_bd *rbd = fec->rbd_base;
 	uint32_t size;
+	void *mem;
 	int i;
 
-	/*
-	 * Allocate memory for the buffers. This allocation respects the
-	 * alignment
-	 */
-	size = roundup(dsize, ARCH_DMA_MINALIGN);
+	if (!rbd->data_pointer) {
+		/*
+		 * Allocate memory for the buffers. This allocation
+		 * respects the alignment.
+		 */
+		size = roundup(dsize, ARCH_DMA_MINALIGN);
+		mem = memalign(ARCH_DMA_MINALIGN, size * count);
+		if (!mem)
+			return -ENOMEM;
+
+		for (i = 0; i < count; i++)
+			rbd[i].data_pointer = (uint32_t)(mem + size * i);
+	}
+
 	for (i = 0; i < count; i++) {
-		uint32_t data_ptr = readl(&fec->rbd_base[i].data_pointer);
-		if (data_ptr == 0) {
-			uint8_t *data = memalign(ARCH_DMA_MINALIGN,
-						 size);
-			if (!data) {
-				printf("%s: error allocating rxbuf %d\n",
-				       __func__, i);
-				goto err;
-			}
-			writel((uint32_t)data, &fec->rbd_base[i].data_pointer);
-		} /* needs allocation */
-		writew(FEC_RBD_EMPTY, &fec->rbd_base[i].status);
-		writew(0, &fec->rbd_base[i].data_length);
+		fec->rbd_base[i].status = FEC_RBD_EMPTY;
+		fec->rbd_base[i].data_length = 0;
 	}
 
 	/* Mark the last RBD to close the ring. */
-	writew(FEC_RBD_WRAP | FEC_RBD_EMPTY, &fec->rbd_base[i - 1].status);
+	fec->rbd_base[count - 1].status |= FEC_RBD_WRAP;
+
 	fec->rbd_index = 0;
 
 	return 0;
-
-err:
-	for (; i >= 0; i--) {
-		uint32_t data_ptr = readl(&fec->rbd_base[i].data_pointer);
-		free((void *)data_ptr);
-	}
-
-	return -ENOMEM;
 }
 
 /**