[U-Boot] [PATCH] net: fec_mxc: Fix timeouts during tftp transfer

[Benoît Thébaudeau]

Hi Marek,

On Tuesday, September 17, 2013 5:00:58 AM, Marek Vasut wrote:
> Dear Fabio Estevam,
> 
> > From: Fabio Estevam <fabio.estevam at freescale.com>
> > 
> > Performing tftp transfers on mx28 results in random timeouts.
> > 
> > Hector Palacios and Robert Hodaszi analyzed the root cause being related to
> > the alignment of the 'buff' buffer inside fec_recv().
> > 
> > GCC versions such as 4.4/4.5 are more likely to exhibit such problem.
> > 
> > Use ALLOC_CACHE_ALIGN_BUFFER() for making the proper alignment of buffer.
> > 
> > Reported-by: Hector Palacios <hector.palacios at digi.com>
> > Tested-by: Oliver Metz <oliver at freetz.org>
> > Signed-off-by: Fabio Estevam <fabio.estevam at freescale.com>
> > ---
> >  drivers/net/fec_mxc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> > index 690e572..b423ff6 100644
> > --- a/drivers/net/fec_mxc.c
> > +++ b/drivers/net/fec_mxc.c
> > @@ -794,7 +794,7 @@ 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);
> > +	ALLOC_CACHE_ALIGN_BUFFER(uchar, buff, FEC_MAX_PKT_SIZE);
> 
> OK, it's gonna be safer this way, still what's the root cause of the issue?
> 
> FEC_MAX_PKT_SIZE is 1536 (which is aligned to 32b and even 64b)
> __aligned(ARCH_DMA_MINALIGN) aligns the stuff to 32b or 64b depending on CPU
> 
> So how can the above not properly align the buffer? Is that a compiler bug
> then?
> 
> btw. using ALLOC_CACHE_ALIGN_BUFFER will be safer were someone to change
> FEC_MAX_PKT_SIZE, the buffer would still be safe for cache flush/inval ops.

I have encountered the same kind of alignment issue recently on something else.
It looks like GCC for ARM just silently ignores the aligned attribute for auto
(stacked) variables.

Best regards,
Benoît