[13/18] dmaengine/amba-pl08x: Align lli_len to max(src.width, dst.width)
diff mbox

Message ID afe9e5154d96c0f44c64c411c98b5db75969db6c.1311936524.git.viresh.kumar@st.com
State New, archived
Headers show

Commit Message

Viresh KUMAR July 29, 2011, 10:49 a.m. UTC
Currently lli_len is aligned to min of two widths, which looks to be incorrect.
Instead it should be aligned to max of both widths.

Lets say, total_size = 441 bytes

MIN: lets check if min() suits or not:

CASE 1: srcwidth = 1, dstwidth = 4
min(src, dst) = 1

i.e. We program transfer size in control reg to 441.
Now, till 440 bytes everything is fine, but on the last byte DMAC can't transfer
1 byte to dst, as its width is 4.

CASE 2: srcwidth = 4, dstwidth = 1
min(src, dst) = 1

i.e. we program transfer size in control reg to 110 (data transferred = 110 * srcwidth).
So, here too 1 byte is left, but on the source side.

MAX: Lets check if max() suits or not:

CASE 3: srcwidth = 1, dstwidth = 4
max(src, dst) = 4

Aligned size is 440

i.e. We program transfer size in control reg to 440.
Now, all 440 bytes will be transferred without any issues.

CASE 4: srcwidth = 4, dstwidth = 1
max(src, dst) = 4

Aligned size is 440

i.e. We program transfer size in control reg to 110 (data transferred = 110 * srcwidth).
Now, also all 440 bytes will be transferred without any issues.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 drivers/dma/amba-pl08x.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

Comments

Vinod Koul July 29, 2011, 12:43 p.m. UTC | #1
On Fri, 2011-07-29 at 16:19 +0530, Viresh Kumar wrote:
> Currently lli_len is aligned to min of two widths, which looks to be incorrect.
> Instead it should be aligned to max of both widths.
hmmm, not sure I follow you...
In previous patch you said

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

So why should lli_len be related to max of both widths?
viresh kumar July 29, 2011, 3:39 p.m. UTC | #2
On 7/29/11, Koul, Vinod <vinod.koul@intel.com> wrote:
> On Fri, 2011-07-29 at 16:19 +0530, Viresh Kumar wrote:
>> Currently lli_len is aligned to min of two widths, which looks to be
>> incorrect.
>> Instead it should be aligned to max of both widths.
> hmmm, not sure I follow you...
> In previous patch you said
>
> "max_bytes_per_lli = bd.srcbus.buswidth *
> PL080_CONTROL_TRANSFER_SIZE_MASK;
> This is confirmed by ARM support guys."
>
> So why should lli_len be related to max of both widths?
>

Sorry if my commit log wasn't clear enough.
Actually there is a field in ctrl reg of channel: transfer_size (it is
max 4095),

total data to be transferred = srcwidth * value programmed in transfer_size.

Now, suppose we have to transfer 98 bytes with src width = HALF WORD
and dest width = WORD

then value to be programmed in transfer_size is 98 / 2 = 49

Now on the destination side, transfers will be word by word and so
after 24 transfers on dest and 48 on src, 96 bytes would be
transferred.

But for the last two bytes, we can read two bytes from src, but can't
transfer them to dest. As its width was 4.

That's why we need to have lli_len to be related to max of both widths?

--
viresh
Linus Walleij July 31, 2011, 12:30 a.m. UTC | #3
2011/7/29 Viresh Kumar <viresh.kumar@st.com>:

> Currently lli_len is aligned to min of two widths, which looks to be incorrect.
> Instead it should be aligned to max of both widths.

Again, a great and well researched patch.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Patch
diff mbox

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 590397d..44f317a 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -662,20 +662,22 @@  static int pl08x_fill_llis_for_desc(struct pl08x_driver_data *pl08x,
 		 * width left
 		 */
 		while (bd.remainder > (mbus->buswidth - 1)) {
-			size_t lli_len, tsize;
+			size_t lli_len, tsize, width;
 
 			/*
 			 * If enough left try to send max possible,
 			 * otherwise try to send the remainder
 			 */
 			lli_len = min(bd.remainder, max_bytes_per_lli);
+
 			/*
-			 * Check against minimum bus alignment: Calculate actual
+			 * Check against maximum bus alignment: Calculate actual
 			 * transfer size in relation to bus width and get a
-			 * maximum remainder of the smallest bus width - 1
+			 * maximum remainder of the highest bus width - 1
 			 */
-			tsize = lli_len / min(mbus->buswidth, sbus->buswidth);
-			lli_len	= tsize * min(mbus->buswidth, sbus->buswidth);
+			width = max(mbus->buswidth, sbus->buswidth);
+			lli_len = (lli_len / width) * width;
+			tsize = lli_len / bd.srcbus.buswidth;
 
 			dev_vdbg(&pl08x->adev->dev,
 					"%s fill lli with single lli chunk of "