[U-Boot,03/51] net: dwc: Fix cache alignment issues

Fix remaining cache alignment issues in the DWC Ethernet driver.
Please note that the cache handling in the driver is making the
code hideous and thus the next patch cleans that up. In order to
make this change reviewable though, the cleanup is split from it.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Chin Liang See <clsee@altera.com>
Cc: Dinh Nguyen <dinguyen@altera.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Tom Rini <trini@ti.com>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Pavel Machek <pavel@denx.de>
Cc: Joe Hershberger <joe.hershberger@gmail.com>
---
 drivers/net/designware.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Hi Marek, 

On Sun, 2014-09-21 at 14:58 +0200, marex@denx.de wrote:
> Fix remaining cache alignment issues in the DWC Ethernet driver.
> Please note that the cache handling in the driver is making the
> code hideous and thus the next patch cleans that up. In order to
> make this change reviewable though, the cleanup is split from it.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Chin Liang See <clsee@altera.com>
> Cc: Dinh Nguyen <dinguyen@altera.com>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Tom Rini <trini@ti.com>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Pavel Machek <pavel@denx.de>
> Cc: Joe Hershberger <joe.hershberger@gmail.com>
> ---
>  drivers/net/designware.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> index 7186e3b..aaf146d 100644
> --- a/drivers/net/designware.c
> +++ b/drivers/net/designware.c
> @@ -303,7 +303,8 @@ static int dw_eth_send(struct eth_device *dev, void *packet, int length)
>  
>  	/* Flush data to be sent */
>  	flush_dcache_range((unsigned long)desc_p->dmamac_addr,
> -			   (unsigned long)desc_p->dmamac_addr + length);
> +			   (unsigned long)desc_p->dmamac_addr +
> +			   roundup(length, ARCH_DMA_MINALIGN));
>  
>  #if defined(CONFIG_DW_ALTDESCRIPTOR)
>  	desc_p->txrx_status |= DESC_TXSTS_TXFIRST | DESC_TXSTS_TXLAST;
> @@ -372,7 +373,8 @@ static int dw_eth_recv(struct eth_device *dev)
>  		/* Flush only status field - others weren't changed */
>  		flush_dcache_range((unsigned long)&desc_p->txrx_status,
>  				   (unsigned long)&desc_p->txrx_status +
> -				   sizeof(desc_p->txrx_status));
> +					roundup(sizeof(desc_p->txrx_status),
> +						ARCH_DMA_MINALIGN));
>  

Current flush_dcache_range will clean & invalidate the whole cache line
which contain the address specified by start and end. Hence I believe
the alignment is not needed. Wonder the existing code got issue?

Thanks
Chin Liang


>  		/* Test the wrap-around condition. */
>  		if (++desc_num >= CONFIG_RX_DESCR_NUM)

On Wednesday, October 01, 2014 at 09:23:51 AM, Chin Liang See wrote:
> Hi Marek,
> 
> On Sun, 2014-09-21 at 14:58 +0200, marex@denx.de wrote:
> > Fix remaining cache alignment issues in the DWC Ethernet driver.
> > Please note that the cache handling in the driver is making the
> > code hideous and thus the next patch cleans that up. In order to
> > make this change reviewable though, the cleanup is split from it.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Chin Liang See <clsee@altera.com>
> > Cc: Dinh Nguyen <dinguyen@altera.com>
> > Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> > Cc: Tom Rini <trini@ti.com>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Pavel Machek <pavel@denx.de>
> > Cc: Joe Hershberger <joe.hershberger@gmail.com>
> > ---
> > 
> >  drivers/net/designware.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> > index 7186e3b..aaf146d 100644
> > --- a/drivers/net/designware.c
> > +++ b/drivers/net/designware.c
> > @@ -303,7 +303,8 @@ static int dw_eth_send(struct eth_device *dev, void
> > *packet, int length)
> > 
> >  	/* Flush data to be sent */
> >  	flush_dcache_range((unsigned long)desc_p->dmamac_addr,
> > 
> > -			   (unsigned long)desc_p->dmamac_addr + length);
> > +			   (unsigned long)desc_p->dmamac_addr +
> > +			   roundup(length, ARCH_DMA_MINALIGN));
> > 
> >  #if defined(CONFIG_DW_ALTDESCRIPTOR)
> >  
> >  	desc_p->txrx_status |= DESC_TXSTS_TXFIRST | DESC_TXSTS_TXLAST;
> > 
> > @@ -372,7 +373,8 @@ static int dw_eth_recv(struct eth_device *dev)
> > 
> >  		/* Flush only status field - others weren't changed */
> >  		flush_dcache_range((unsigned long)&desc_p->txrx_status,
> >  		
> >  				   (unsigned long)&desc_p->txrx_status +
> > 
> > -				   sizeof(desc_p->txrx_status));
> > +					roundup(sizeof(desc_p->txrx_status),
> > +						ARCH_DMA_MINALIGN));
> 
> Current flush_dcache_range will clean & invalidate the whole cache line
> which contain the address specified by start and end. Hence I believe
> the alignment is not needed. Wonder the existing code got issue?

Don't be mistaken, please see drivers/net/designware.h :
sizeof(desc_p->txrx_status) == sizeof(u32) == 4

Thus the flush is called like so:
flush_dcache_range(<address of start of DMA descriptor>,
                   <address of start of DMA descriptor> + 4);

On ARM, the cache ops have to be aligned to cacheline length, so the above code 
will indeed have issues. Thus the alignment.

The subsequent patch just converts this whole mess to flushing the entire 
descriptor , which is much more sane approach to take.

Best regards,
Marek Vasut