All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Hodaszi <robert.hodaszi@digi.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] net: fec: Avoid MX28 bus sync issue
Date: Fri, 13 Sep 2013 13:13:38 +0200	[thread overview]
Message-ID: <5232F362.60704@digi.com> (raw)
In-Reply-To: <5232F2E7.4050407@digi.com>


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

  reply	other threads:[~2013-09-13 11:13 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-11 23:03 [U-Boot] [PATCH] net: fec: Avoid MX28 bus sync issue Marek Vasut
2013-07-11 23:18 ` Fabio Estevam
2013-07-12  3:41   ` Alexandre Pereira da Silva
2013-07-12  3:51     ` Marek Vasut
2013-07-12 11:37       ` Hector Palacios
2013-07-12 11:39         ` Fabio Estevam
2013-07-12 12:01         ` Marek Vasut
2013-07-12 15:08           ` Hector Palacios
2013-07-12 15:50             ` Albert ARIBAUD
2013-07-12 16:48             ` Marek Vasut
2013-07-15  8:58               ` Hector Palacios
2013-07-15 12:30                 ` Marek Vasut
2013-07-15 15:09                   ` Hector Palacios
2013-07-15 15:12                     ` Marek Vasut
2013-07-15 15:24                       ` Hector Palacios
2013-07-16  3:51                     ` Fabio Estevam
2013-07-16  4:18                       ` Fabio Estevam
2013-07-16  4:44                         ` Marek Vasut
2013-07-17 15:55                           ` Hector Palacios
2013-07-18  4:12                             ` Marek Vasut
2013-09-12 10:22                             ` Hector Palacios
2013-09-12 10:50                               ` Marek Vasut
     [not found]                                 ` <52319DE8.5080607@digi.com>
2013-09-12 11:00                                   ` Marek Vasut
2013-09-12 11:02                                 ` Robert Hodaszi
2013-09-12 14:05                                   ` Marek Vasut
2013-09-12 14:15                                     ` Robert Hodaszi
2013-09-12 14:31                                       ` Marek Vasut
2013-09-12 14:32                                         ` Robert Hodaszi
2013-09-12 15:06                                           ` Marek Vasut
2013-09-12 18:17                                     ` Wolfgang Denk
2013-09-12 18:39                                       ` Fabio Estevam
2013-09-12 18:53                                         ` Wolfgang Denk
2013-09-12 19:37                                           ` Fabio Estevam
2013-09-13 11:11                                             ` Robert Hodaszi
2013-09-13 11:13                                               ` Robert Hodaszi [this message]
2013-09-13 14:01                                                 ` Marek Vasut
2013-09-13 14:24                                                   ` Robert Hodaszi
2013-09-13 16:06                                               ` Wolfgang Denk
2013-09-13 16:24                                                 ` Marek Vasut
2013-09-13 17:46                                                   ` Wolfgang Denk
2013-09-14 22:05                                                     ` Fabio Estevam
2013-09-12 11:08                                 ` Robert Hodaszi
2013-09-12 18:12                                   ` Wolfgang Denk
2013-09-12 17:50                               ` Wolfgang Denk
2013-07-13  2:43   ` Troy Kisky
2013-07-15 13:41     ` Albert ARIBAUD
2013-07-15 17:39       ` Troy Kisky
2013-07-15 19:59         ` Troy Kisky
2013-07-15 20:20           ` Albert ARIBAUD
2013-07-15 20:20         ` Albert ARIBAUD
2013-07-15 21:18           ` Troy Kisky
2013-07-12  5:57 ` Albert ARIBAUD
2013-07-12  6:39   ` Albert ARIBAUD
2013-07-12 11:51     ` Marek Vasut
2013-07-12  6:56   ` Stefano Babic
2013-07-12  7:30 ` Stefano Babic
2013-09-15 18:12 Oliver Metz
2013-09-15 18:16 ` Fabio Estevam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5232F362.60704@digi.com \
    --to=robert.hodaszi@digi.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.