[U-Boot] mmc: dcache: Replace ext_csd buffer with cache aligned one

This commit replaces the ext_csd buffer allocated as an automatic
variable with one cache aligned. The ext_csd might be allocated with
alignment not equal to the L1 D cache alignment.

The memalign from common/dlmalloc.c is allowing for buffer allocation
with proper cache alignment.

The common/dlmalloc.c [c|m]alloc alignment is hardwired to 8 bytes,
so out of the box functions cannot be safely used with L1 D cache.

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Andy Fleming <afleming@gmail.com>
CC: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
 drivers/mmc/mmc.c |   55 +++++++++++++++++++++++++++++++++-------------------
 1 files changed, 35 insertions(+), 20 deletions(-)

Hi Lukasz,

On 12/08/2011 10:25, Lukasz Majewski wrote:

> +	char *ext_csd = memalign(get_dcache_line_size(), mmc->read_bl_len);

Since this is the first effort for aligning buffers on the caller side, 
I'd like to make sure the method can be, and is, standardized across all 
changes.

So before standardizing on memalign() for dynamically allocating aligned 
buffers, I want to to be sure that the use of this function is portable.

I know it is in GNU LIBC; I also know it is not (yet...) in U-Boot's own 
C library. What about the most common toolchains used on U-Boot?

Amicalement,

Hi Albert,

On Fri, 12 Aug 2011 11:07:57 +0200
Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:

> I know it is in GNU LIBC; I also know it is not (yet...) in U-Boot's
> own C library. What about the most common toolchains used on U-Boot?

The memalign is already defined in the u-boot tree (common/dlmalloc.c).

The dlmalloc.o is also built during compilation and it is linked to the
final u-boot binary.

I'm using the CodeSourgery's ARM toolchain (gcc version 4.4.1 (Sourcery
G++ Lite 2009q3-68)). 
I can test it with (gcc version 4.3.2 (Sourcery G++ Lite 2008q3-72)) as
well.

Moreover I can try to install OSELAS.Toolchain (PTXdist ones) and test
this as well with those toolchains. There are several one available for
armv5/armv6/armv7.

Initially I was planning to use calloc/malloc from ./common/dlmalloc.c
but it is clearly stated, that it is using 8 bytes alignment (which is
hardwired in this implementation). 

I will keep you informed about the tests results.

Hi Lukasz,

On 12/08/2011 11:35, Lukasz Majewski wrote:
> Hi Albert,
>
> On Fri, 12 Aug 2011 11:07:57 +0200
> Albert ARIBAUD<albert.u.boot@aribaud.net>  wrote:
>
>> I know it is in GNU LIBC; I also know it is not (yet...) in U-Boot's
>> own C library. What about the most common toolchains used on U-Boot?
>
> The memalign is already defined in the u-boot tree (common/dlmalloc.c).

Apologies: seems like I missed it.

> The dlmalloc.o is also built during compilation and it is linked to the
> final u-boot binary.
>
> I'm using the CodeSourgery's ARM toolchain (gcc version 4.4.1 (Sourcery
> G++ Lite 2009q3-68)).
> I can test it with (gcc version 4.3.2 (Sourcery G++ Lite 2008q3-72)) as
> well.
>
> Moreover I can try to install OSELAS.Toolchain (PTXdist ones) and test
> this as well with those toolchains. There are several one available for
> armv5/armv6/armv7.
>
> Initially I was planning to use calloc/malloc from ./common/dlmalloc.c
> but it is clearly stated, that it is using 8 bytes alignment (which is
> hardwired in this implementation).
>
> I will keep you informed about the tests results.

Thanks a lot! If no toolchain gives issues, then I guess the use of 
memalign() for dynamic buffers can be considered the way to go.

Amicalement,

On Fri, Aug 12, 2011 at 3:25 AM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> This commit replaces the ext_csd buffer allocated as an automatic
> variable with one cache aligned. The ext_csd might be allocated with
> alignment not equal to the L1 D cache alignment.
>
> The memalign from common/dlmalloc.c is allowing for buffer allocation
> with proper cache alignment.
>
> The common/dlmalloc.c [c|m]alloc alignment is hardwired to 8 bytes,
> so out of the box functions cannot be safely used with L1 D cache.
>
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> CC: Andy Fleming <afleming@gmail.com>
> CC: Albert ARIBAUD <albert.u.boot@aribaud.net>
> ---
>  drivers/mmc/mmc.c |   55 +++++++++++++++++++++++++++++++++-------------------
>  1 files changed, 35 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index 7e703c0..422a0f9 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -660,7 +664,11 @@ int mmc_change_freq(struct mmc *mmc)
>        else
>                mmc->card_caps |= MMC_MODE_HS;
>
> -       return 0;
> + exit:
> +       err = 0;


Just initialize err to 0 at the beginning of the function, and relabel
"error" as "out"


> + error:
> +       free(ext_csd);
> +       return err;
>  }
>
>  int mmc_switch_part(int dev_num, unsigned int part_num)

> @@ -1122,6 +1134,9 @@ int mmc_startup(struct mmc *mmc)
>        init_part(&mmc->block_dev);
>
>        return 0;
> + error:
> +       free(ext_csd);

Need to remove that "return 0" so that ext_csd gets freed if things go
correctly, too. I would also relabel this one as "out", so as to not
confuse the casual reader.

Andy