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

Robert Hodaszi
2013-09-13T11:13:38Z

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 happened
>
>
>
> Best regards,
> Robert Hodaszi
>

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

Best regards,
Robert Hodaszi