[11/18] dmaengine/amba-pl08x: max_bytes_per_lli is TRANSFER_SIZE * src_width (not MIN(width))

Commit Message

Viresh KUMAR July 29, 2011, 10:49 a.m. UTC

max_bytes_per_lli = bd.srcbus.buswidth * PL080_CONTROL_TRANSFER_SIZE_MASK;
This is confirmed by ARM support guys.

Below is summary of mail exchange with them:

[Viresh] What is the total data to be transferred in case source and destination
bus widths are different. Suppose, source bus width is 2 bytes and destination
is 4 bytes. Now in order to transfer 80 bytes, what should be value of
TransferSize field in control reg: 40? or 20?.

[David from ARM] The value that is programmed into the TransferSize field should
be the number of <SourceWidth> transfers needed to achieve the required data
transfer.

So, to transfer 80 bytes, with a Source Width of 2, the TransferSize field =
should be programmed with:

        Total transfer size
        ------------------- = 40
          <source width>

[Viresh] Will this change if source is 4 bytes and dest is 2?

[David] Yes - the calculation then becomes:

        Total transfer size
        ------------------- =20
          <source width>

Also, max_bytes_per_lli must be calculated after fixing src and dest widths not
before that. So move this code to the correct place.

This patch also removes max_bytes_per_lli from earlier print message, as till
that point max_bytes_per_lli is unknown.

Signed-off-by: Viresh Kumar <[e-mail]>
---
 drivers/dma/amba-pl08x.c |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)

Comments

Linus Walleij July 31, 2011, 12:27 a.m. UTC | #1

2011/7/29 Viresh Kumar <[e-mail]>:

> max_bytes_per_lli = bd.srcbus.buswidth * PL080_CONTROL_TRANSFER_SIZE_MASK;
> This is confirmed by ARM support guys.

Great and THANKS for your tiresome research in ironing out the ambiguities
in the manuals.

Acked-by: Linus Walleij <[e-mail]>

Yours,
Linus Walleij

Russell King - ARM Linux July 31, 2011, 9:09 a.m. UTC | #2

On Sun, Jul 31, 2011 at 02:27:38AM +0200, Linus Walleij wrote:
> 2011/7/29 Viresh Kumar <[e-mail]>:
> 
> > max_bytes_per_lli = bd.srcbus.buswidth * PL080_CONTROL_TRANSFER_SIZE_MASK;
> > This is confirmed by ARM support guys.
> 
> Great and THANKS for your tiresome research in ironing out the ambiguities
> in the manuals.

The manual isn't that bad.  The code from ARM though is, and raises lots
of "why are they doing that?  oh, they must know better than the docs"
type questions.  Obviously they don't!

Patch

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 04f7889..3b7357e 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -597,24 +597,17 @@  static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
 	bd.srcbus.buswidth = bd.srcbus.maxwidth;
 	bd.dstbus.buswidth = bd.dstbus.maxwidth;
 
-	/*
-	 * Bytes transferred == tsize * MIN(buswidths), not max(buswidths)
-	 */
-	max_bytes_per_lli = min(bd.srcbus.buswidth, bd.dstbus.buswidth) *
-		PL080_CONTROL_TRANSFER_SIZE_MASK;
-
 	/* We need to count this down to zero */
 	bd.remainder = txd->len;
 
 	pl08x_choose_master_bus(&bd, &mbus, &sbus, cctl);
 
-	dev_vdbg(&pl08x->adev->dev, "src=0x%08x%s/%u dst=0x%08x%s/%u len=%zu "
-			"llimax=%zu\n",
+	dev_vdbg(&pl08x->adev->dev, "src=0x%08x%s/%u dst=0x%08x%s/%u len=%zu\n",
 		 bd.srcbus.addr, cctl & PL080_CONTROL_SRC_INCR ? "+" : "",
 		 bd.srcbus.buswidth,
 		 bd.dstbus.addr, cctl & PL080_CONTROL_DST_INCR ? "+" : "",
 		 bd.dstbus.buswidth,
-		 bd.remainder, max_bytes_per_lli);
+		 bd.remainder);
 	dev_vdbg(&pl08x->adev->dev, "mbus=%s sbus=%s\n",
 		 mbus == &bd.srcbus ? "src" : "dst",
 		 sbus == &bd.srcbus ? "src" : "dst");
@@ -654,6 +647,10 @@  static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
 			sbus->buswidth = 1;
 		}
 
+		/* Bytes transferred = tsize * src width, not MIN(buswidths) */
+		max_bytes_per_lli = bd.srcbus.buswidth *
+			PL080_CONTROL_TRANSFER_SIZE_MASK;
+
 		/*
 		 * Make largest possible LLIs until less than one bus
 		 * width left