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

[Robert Hodaszi]

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 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
>

I think, I sent it again in HTML format... Sorry...


Best regards,
Robert Hodaszi