[U-Boot] [PATCH] net: fec: Avoid MX28 bus sync issue

[Robert Hodaszi]

On 2013-09-12 21:37, Fabio Estevam wrote:
> On Thu, Sep 12, 2013 at 3:53 PM, Wolfgang Denk <wd at denx.de> wrote:
>> Dear Fabio Estevam,
>>
>> In message <CAOMZO5BY6kDOCoWn_SRwhPE7ssMjAReZ2bcFXGgFF-_Wrdgo0g at 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 at 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 at 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 happened



Best regards,
Robert Hodaszi