From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robert Hodaszi Date: Fri, 13 Sep 2013 13:13:38 +0200 Subject: [U-Boot] [PATCH] net: fec: Avoid MX28 bus sync issue In-Reply-To: <5232F2E7.4050407@digi.com> References: <1373583784-7129-1-git-send-email-marex@denx.de> <201309121250.36579.marex@denx.de> <52319F63.2030409@digi.com> <201309121605.04824.marex@denx.de> <20130912181757.5F102380189@gemini.denx.de> <20130912185349.9743B380189@gemini.denx.de> <5232F2E7.4050407@digi.com> Message-ID: <5232F362.60704@digi.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 wrote: >>> Dear Fabio Estevam, >>> >>> In message >>> >>> 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 > 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 > --- > 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