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

[Marek Vasut]

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

OK, but as Wolfgang already pointed out, can you tell use what compiler exposes 
this behavior and show us the details WD requested please ?

Best regards,
Marek Vasut