[U-Boot,06/51] mmc: dw_mmc: Fix cache alignment issue

The DMA descriptors used by the DW MMC block must be aligned to cacheline
size, otherwise we are unable to properly flush/inval cache over them and
we get data corruption.

The reason I chose this approach of expanding the structure is because
the driver allocates the descriptors in bulk. This approach does waste
space by inserting slop inbetween the descriptors, but it makes access
to the descriptors easy as the compiler does know the real size of the
structure. It also makes cache operations easy, since the size of the
structure is cache aligned and the structure start address is as well.

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: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 include/dwmmc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi Marek,

On Sun, 2014-09-21 at 14:58 +0200, marex@denx.de wrote:
> The DMA descriptors used by the DW MMC block must be aligned to cacheline
> size, otherwise we are unable to properly flush/inval cache over them and
> we get data corruption.
> 
> The reason I chose this approach of expanding the structure is because
> the driver allocates the descriptors in bulk. This approach does waste
> space by inserting slop inbetween the descriptors, but it makes access
> to the descriptors easy as the compiler does know the real size of the
> structure. It also makes cache operations easy, since the size of the
> structure is cache aligned and the structure start address is as well.
> 
> 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: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  include/dwmmc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/dwmmc.h b/include/dwmmc.h
> index b67f11b..109f7c8 100644
> --- a/include/dwmmc.h
> +++ b/include/dwmmc.h
> @@ -157,7 +157,7 @@ struct dwmci_idmac {
>  	u32 cnt;
>  	u32 addr;
>  	u32 next_addr;
> -};
> +} __aligned(ARCH_DMA_MINALIGN);
>  

Wonder the ALLOC_CACHE_ALIGN_BUFFER within function dwmci_send_cmd at
drivers/mmc/dw_mmc.c already take care this?

Thanks
Chin Liang

>  static inline void dwmci_writel(struct dwmci_host *host, int reg, u32 val)
>  {

On Wednesday, October 01, 2014 at 11:45:30 AM, Chin Liang See wrote:
> Hi Marek,
> 
> On Sun, 2014-09-21 at 14:58 +0200, marex@denx.de wrote:
> > The DMA descriptors used by the DW MMC block must be aligned to cacheline
> > size, otherwise we are unable to properly flush/inval cache over them and
> > we get data corruption.
> > 
> > The reason I chose this approach of expanding the structure is because
> > the driver allocates the descriptors in bulk. This approach does waste
> > space by inserting slop inbetween the descriptors, but it makes access
> > to the descriptors easy as the compiler does know the real size of the
> > structure. It also makes cache operations easy, since the size of the
> > structure is cache aligned and the structure start address is as well.
> > 
> > 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: Pantelis Antoniou <panto@antoniou-consulting.com>
> > ---
> > 
> >  include/dwmmc.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/dwmmc.h b/include/dwmmc.h
> > index b67f11b..109f7c8 100644
> > --- a/include/dwmmc.h
> > +++ b/include/dwmmc.h
> > @@ -157,7 +157,7 @@ struct dwmci_idmac {
> > 
> >  	u32 cnt;
> >  	u32 addr;
> >  	u32 next_addr;
> > 
> > -};
> > +} __aligned(ARCH_DMA_MINALIGN);
> 
> Wonder the ALLOC_CACHE_ALIGN_BUFFER within function dwmci_send_cmd at
> drivers/mmc/dw_mmc.c already take care this?

This won't help, since you want to do cache ops only on one descriptor at a 
time. The ALLOC_CACHE_ALIGN_BUFFER will allocate an array of the descriptors
with aligned start address and end address, but won't insert the necessary 
padding inbetween the elements so that each descriptor starts at the begining
of a cacheline. So you can do:

flush_dcache_range((u32)desc, (u32)desc + sizeof(*desc));

where $desc is a pointer to the n-th element in the array of descriptors.

I explained in the commit message why I didn't go for a scheme where the 
descriptors would be tightly packed one after the other -- it is possible
to do it, but the already obscure cache handling mechanism would be even
more complex that way and thus more bug-prone.

Best regards,
Marek Vasut