[U-Boot] [PATCH] 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(-)
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 @@ -29,6 +29,7 @@ #include <mmc.h> #include <part.h> #include <malloc.h> +#include <errno.h> #include <linux/list.h> #include <div64.h>
@@ -617,42 +618,45 @@ int mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value)
int mmc_change_freq(struct mmc *mmc) { - char ext_csd[512]; char cardtype; int err; + char *ext_csd = memalign(get_dcache_line_size(), mmc->read_bl_len); + + if (!ext_csd) + return -ENOMEM;
mmc->card_caps = 0;
if (mmc_host_is_spi(mmc)) - return 0; + goto exit;
/* Only version 4 supports high-speed */ if (mmc->version < MMC_VERSION_4) - return 0; + goto exit;
mmc->card_caps |= MMC_MODE_4BIT;
err = mmc_send_ext_csd(mmc, ext_csd);
if (err) - return err; + goto error;
cardtype = ext_csd[196] & 0xf;
err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1);
if (err) - return err; + goto error;
/* Now check to see that it worked */ err = mmc_send_ext_csd(mmc, ext_csd);
if (err) - return err; + goto error;
/* No high-speed support */ if (!ext_csd[185]) - return 0; + goto exit;
/* High Speed is set, there are two types: 52MHz and 26MHz */ if (cardtype & MMC_HS_52MHZ) @@ -660,7 +664,11 @@ int mmc_change_freq(struct mmc *mmc) else mmc->card_caps |= MMC_MODE_HS;
- return 0; + exit: + err = 0; + error: + free(ext_csd); + return err; }
int mmc_switch_part(int dev_num, unsigned int part_num) @@ -855,11 +863,15 @@ void mmc_set_bus_width(struct mmc *mmc, uint width)
int mmc_startup(struct mmc *mmc) { - int err; + int err, i; uint mult, freq; u64 cmult, csize, capacity; struct mmc_cmd cmd; - char ext_csd[512]; + char *ext_csd = memalign(get_dcache_line_size(), mmc->read_bl_len); + + if (!ext_csd) + return -ENOMEM; + int timeout = 1000;
#ifdef CONFIG_MMC_SPI_CRC_ON @@ -871,7 +883,7 @@ int mmc_startup(struct mmc *mmc) err = mmc_send_cmd(mmc, &cmd, NULL);
if (err) - return err; + goto error; } #endif
@@ -885,7 +897,7 @@ int mmc_startup(struct mmc *mmc) err = mmc_send_cmd(mmc, &cmd, NULL);
if (err) - return err; + goto error;
memcpy(mmc->cid, cmd.response, 16);
@@ -903,7 +915,7 @@ int mmc_startup(struct mmc *mmc) err = mmc_send_cmd(mmc, &cmd, NULL);
if (err) - return err; + goto error;
if (IS_SD(mmc)) mmc->rca = (cmd.response[0] >> 16) & 0xffff; @@ -921,7 +933,7 @@ int mmc_startup(struct mmc *mmc) mmc_send_status(mmc, timeout);
if (err) - return err; + goto error;
mmc->csd[0] = cmd.response[0]; mmc->csd[1] = cmd.response[1]; @@ -994,7 +1006,7 @@ int mmc_startup(struct mmc *mmc) err = mmc_send_cmd(mmc, &cmd, NULL);
if (err) - return err; + goto error; }
/* @@ -1044,7 +1056,7 @@ int mmc_startup(struct mmc *mmc) err = mmc_change_freq(mmc);
if (err) - return err; + goto error;
/* Restrict card's capabilities by what the host can do */ mmc->card_caps &= mmc->host_caps; @@ -1058,7 +1070,7 @@ int mmc_startup(struct mmc *mmc)
err = mmc_send_cmd(mmc, &cmd, NULL); if (err) - return err; + goto error;
cmd.cmdidx = SD_CMD_APP_SET_BUS_WIDTH; cmd.resp_type = MMC_RSP_R1; @@ -1066,7 +1078,7 @@ int mmc_startup(struct mmc *mmc) cmd.flags = 0; err = mmc_send_cmd(mmc, &cmd, NULL); if (err) - return err; + goto error;
mmc_set_bus_width(mmc, 4); } @@ -1083,7 +1095,7 @@ int mmc_startup(struct mmc *mmc) EXT_CSD_BUS_WIDTH_4);
if (err) - return err; + goto error;
mmc_set_bus_width(mmc, 4); } else if (mmc->card_caps & MMC_MODE_8BIT) { @@ -1093,7 +1105,7 @@ int mmc_startup(struct mmc *mmc) EXT_CSD_BUS_WIDTH_8);
if (err) - return err; + goto error;
mmc_set_bus_width(mmc, 8); } @@ -1122,6 +1134,9 @@ int mmc_startup(struct mmc *mmc) init_part(&mmc->block_dev);
return 0; + error: + free(ext_csd); + return err; }
int mmc_send_if_cond(struct mmc *mmc)
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 ARIBAUDalbert.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
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 --- Changes for v2: - Code cleanup - "out" label instead of "error" --- drivers/mmc/mmc.c | 56 ++++++++++++++++++++++++++++++++-------------------- 1 files changed, 34 insertions(+), 22 deletions(-)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 7e703c0..5f79a17 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -29,6 +29,7 @@ #include <mmc.h> #include <part.h> #include <malloc.h> +#include <errno.h> #include <linux/list.h> #include <div64.h>
@@ -617,42 +618,45 @@ int mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value)
int mmc_change_freq(struct mmc *mmc) { - char ext_csd[512]; char cardtype; - int err; + int err = 0; + char *ext_csd = memalign(get_dcache_line_size(), mmc->read_bl_len); + + if (!ext_csd) + return -ENOMEM;
mmc->card_caps = 0;
if (mmc_host_is_spi(mmc)) - return 0; + goto out;
/* Only version 4 supports high-speed */ if (mmc->version < MMC_VERSION_4) - return 0; + goto out;
mmc->card_caps |= MMC_MODE_4BIT;
err = mmc_send_ext_csd(mmc, ext_csd);
if (err) - return err; + goto out;
cardtype = ext_csd[196] & 0xf;
err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, 1);
if (err) - return err; + goto out;
/* Now check to see that it worked */ err = mmc_send_ext_csd(mmc, ext_csd);
if (err) - return err; + goto out;
/* No high-speed support */ if (!ext_csd[185]) - return 0; + goto out;
/* High Speed is set, there are two types: 52MHz and 26MHz */ if (cardtype & MMC_HS_52MHZ) @@ -660,7 +664,9 @@ int mmc_change_freq(struct mmc *mmc) else mmc->card_caps |= MMC_MODE_HS;
- return 0; + out: + free(ext_csd); + return err; }
int mmc_switch_part(int dev_num, unsigned int part_num) @@ -855,11 +861,15 @@ void mmc_set_bus_width(struct mmc *mmc, uint width)
int mmc_startup(struct mmc *mmc) { - int err; + int err = 0; uint mult, freq; u64 cmult, csize, capacity; struct mmc_cmd cmd; - char ext_csd[512]; + char *ext_csd = memalign(get_dcache_line_size(), mmc->read_bl_len); + + if (!ext_csd) + return -ENOMEM; + int timeout = 1000;
#ifdef CONFIG_MMC_SPI_CRC_ON @@ -871,7 +881,7 @@ int mmc_startup(struct mmc *mmc) err = mmc_send_cmd(mmc, &cmd, NULL);
if (err) - return err; + goto out; } #endif
@@ -885,7 +895,7 @@ int mmc_startup(struct mmc *mmc) err = mmc_send_cmd(mmc, &cmd, NULL);
if (err) - return err; + goto out;
memcpy(mmc->cid, cmd.response, 16);
@@ -903,7 +913,7 @@ int mmc_startup(struct mmc *mmc) err = mmc_send_cmd(mmc, &cmd, NULL);
if (err) - return err; + goto out;
if (IS_SD(mmc)) mmc->rca = (cmd.response[0] >> 16) & 0xffff; @@ -921,7 +931,7 @@ int mmc_startup(struct mmc *mmc) mmc_send_status(mmc, timeout);
if (err) - return err; + goto out;
mmc->csd[0] = cmd.response[0]; mmc->csd[1] = cmd.response[1]; @@ -994,7 +1004,7 @@ int mmc_startup(struct mmc *mmc) err = mmc_send_cmd(mmc, &cmd, NULL);
if (err) - return err; + goto out; }
/* @@ -1044,7 +1054,7 @@ int mmc_startup(struct mmc *mmc) err = mmc_change_freq(mmc);
if (err) - return err; + goto out;
/* Restrict card's capabilities by what the host can do */ mmc->card_caps &= mmc->host_caps; @@ -1058,7 +1068,7 @@ int mmc_startup(struct mmc *mmc)
err = mmc_send_cmd(mmc, &cmd, NULL); if (err) - return err; + goto out;
cmd.cmdidx = SD_CMD_APP_SET_BUS_WIDTH; cmd.resp_type = MMC_RSP_R1; @@ -1066,7 +1076,7 @@ int mmc_startup(struct mmc *mmc) cmd.flags = 0; err = mmc_send_cmd(mmc, &cmd, NULL); if (err) - return err; + goto out;
mmc_set_bus_width(mmc, 4); } @@ -1083,7 +1093,7 @@ int mmc_startup(struct mmc *mmc) EXT_CSD_BUS_WIDTH_4);
if (err) - return err; + goto out;
mmc_set_bus_width(mmc, 4); } else if (mmc->card_caps & MMC_MODE_8BIT) { @@ -1093,7 +1103,7 @@ int mmc_startup(struct mmc *mmc) EXT_CSD_BUS_WIDTH_8);
if (err) - return err; + goto out;
mmc_set_bus_width(mmc, 8); } @@ -1121,7 +1131,9 @@ int mmc_startup(struct mmc *mmc) (mmc->cid[2] >> 24) & 0xf); init_part(&mmc->block_dev);
- return 0; + out: + free(ext_csd); + return err; }
int mmc_send_if_cond(struct mmc *mmc)
participants (3)
-
Albert ARIBAUD -
Andy Fleming -
Lukasz Majewski