On 2013-09-13 13:11, Robert Hodaszi wrote:
On 2013-09-12 21:37, Fabio Estevam wrote:
On Thu, Sep 12, 2013 at 3:53 PM, Wolfgang Denk wd@denx.de wrote:
Dear Fabio Estevam,
In message CAOMZO5BY6kDOCoWn_SRwhPE7ssMjAReZ2bcFXGgFF-_Wrdgo0g@mail.gmail.com you wrote:
It makes perfect sense to allocate variable with function scope only on the stack. That's what the stack has been invented for.
This buffer in the fec driver will be used in DMA transfer, so maybe that's the reason we should use malloc instead of using the stack?
What has DMA to do with that? We're talking about alignment only.
I mentioned DMA because we align the buffer with __aligned(ARCH_DMA_MINALIGN).
Will try to see if I can reproduce the problem here, but the last time I tried I was not able to.
Maybe the gcc version that Robert and Hector pointed out may explain the different behaviour.
Regards,
Fabio Estevam
Ok. Then what about if I would use the stack, but align the buffer manually.
From: Robert Hodaszi robert.hodaszi@digi.com Date: Fri, 13 Sep 2013 13:07:52 +0200 Subject: [PATCH] net: fec: fix invalid temporary RX buffer alignment because of GCC bug
Older GCC versions don't handle well alignment on stack variables. The temporary RX buffer is a local variable, so it is on the stack. Because the FEC driver is using DMA for transmission, receive and transmit buffers should be align on 64 byte. The transmit buffers are not allocated in the driver internally, it sends the packets directly as it gets them. So these packets should be aligned. When the ARP layer wants to reply to an ARP request, it uses the FEC driver's temporary RX buffer (used to pass data to the ARP layer) to store the new packet, and pass it back to the FEC driver's send function. Because of a GCC bug, this buffer is not aligned well, and when the driver tries to send it, it first rounds the address down to the alignment boundary. That causes invalid data.
To fix it, allocate more space on the stack, and align the buffer manually.
Signed-off-by: Robert Hodaszi robert.hodaszi@digi.com
drivers/net/fec_mxc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index f4f72b7..09d816d 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -828,7 +828,9 @@ static int fec_recv(struct eth_device *dev) uint16_t bd_status; uint32_t addr, size, end; int i;
uchar buff[FEC_MAX_PKT_SIZE] __aligned(ARCH_DMA_MINALIGN);
/* Align the receive buffer */uchar buff_unaligned[FEC_MAX_PKT_SIZE + (ARCH_DMA_MINALIGN - 1)];uchar *buff = ((uint32_t)buff_unaligned + (ARCH_DMA_MINALIGN -1)) & ~(ARCH_DMA_MINALIGN - 1);
/* * Check if any critical events have happenedBest regards, Robert Hodaszi
I think, I sent it again in HTML format... Sorry...
Best regards, Robert Hodaszi