Thread (36 messages) 36 messages, 6 authors, 2011-10-10

Re: [PATCH] mlx4_en: fix transmit of packages when blue frame is enabled

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2011-10-09 08:01:17
Also in: linuxppc-dev

On Sun, 2011-10-09 at 09:35 +0200, Eli Cohen wrote:
On Sun, Oct 09, 2011 at 09:25:18AM +0200, Benjamin Herrenschmidt wrote:
quoted
On Thu, 2011-10-06 at 15:57 +0200, Eli Cohen wrote:
quoted
On Wed, Oct 05, 2011 at 10:15:02AM +0200, Eli Cohen wrote:

How about this patch - can you give it a try?

quoted
From dee60547aa9e35a02835451d9e694cd80dd3072f Mon Sep 17 00:00:00 2001
From: Eli Cohen <redacted>
Date: Thu, 6 Oct 2011 15:50:02 +0200
Subject: [PATCH] mlx4_en: Fix blue flame on powerpc

The source buffer used for copying into the blue flame register is already in
big endian. However, when copying to device on powerpc, the endianess is
swapped so the data reaches th device in little endian which is wrong. On x86
based platform no swapping occurs so it reaches the device with the correct
endianess. Fix this by calling le32_to_cpu() on the buffer. On LE systems there
is no change; on BE there will be a swap.
That looks wrong.
Not sure I understand: are you saying that on ppc, when you call
__iowrite64_copy, it will not reach the device swapped?
Well, first, what do you mean by "swapped" ? :-) But no, it won't for
all intend and purpose, this is a copy routine, copy routines never
swap, neither do fifo accesses for example.
The point is that we must always have the buffer ready in big endian
in memory. In the case of blue flame, we must also copy it to the
device registers in pci memory space. So if we use the buffer we
already prepared, we must have another swap. I can think of a nicer
way to implement this functionality but the question is do you think
my observation above is wrong and why.
No. If it's in memory BE then the copy routine will keep it BE. A copy
routine doesn't swap and doesn't affect endianness.

Additionally, a swapping phase like you proposed doing 32-bit swaps
means that you know for sure that the buffer is made of 32-bit
quantities, is that the case ? Even if you had needed that swap, if your
buffer had contained 16-bit or 64-bit quantities, you're toast.

What is this buffer anyway ? A descriptor or a network packet ?

If it's a packet, then it's data, endianness has no meaning (or rather
it has for individual fields of the packets but they are already in the
right format and a 32-bit swap will never be right).
 
It's almost never right to perform swapping when copying data (or
reading/writing a FIFO).

Cheers,
Ben.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help