mmc: dcache: Replace ext_csd buffer with cache aligned one
PATCH 1/1 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.
--- drivers/mmc/mmc.c | 55 +++++++++++++++++++++++++++++++++------------------- 1 files changed, 35 insertions(+), 20 deletions(-)
drivers/mmc/mmc.c +35 -20
@@ -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)
Signature · 1 line
1.7.2.3
PATCH 1/1 mmc: dcache: Replace ext_csd buffer with cache aligned one
Hi Lukasz,
Review
Quoted 1 line · Lukasz Majewski
(quoted diff) +1
-0+ 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,
Signature · 1 line
Albert.
PATCH 1/1 mmc: dcache: Replace ext_csd buffer with cache aligned one
Hi Albert,
Quoted 2 lines · On Fri, 12 Aug 2011 11:07:57 +0200 Albert ARIBAUD <[email protected]>…
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.
Signature · 6 lines
Best regards, Lukasz Majewski Samsung Poland R&D Center Platform Group
PATCH 1/1 mmc: dcache: Replace ext_csd buffer with cache aligned one
Hi Lukasz,
Quoted 9 lines · Lukasz Majewski
Hi Albert,
Quoted 2 lines · On Fri, 12 Aug 2011 11:07:57 +0200 Albert ARIBAUD<[email protected]>…
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.
Quoted 12 lines
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.
Quoted 5 lines
Initially I was planning to use calloc/malloc from
./common/dlmalloc.cbut 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,
Signature · 1 line
Albert.
PATCH 1/1 mmc: dcache: Replace ext_csd buffer with cache aligned one
In reply to Lukasz MajewskiQuoted 26 lines · Lukasz Majewski
This commit replaces the
ext_csdbuffer allocated as an automatic variable with one cache aligned. Theext_csdmight be allocated with alignment not equal to the L1 D cache alignment.The memalign from
common/dlmalloc.cis 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 <[email protected]>Signed-off-by Kyungmin Park <[email protected]>CC Andy Fleming <[email protected]>CC Albert ARIBAUD <[email protected]>---
?drivers/mmc/mmc.c | ? 55 +++++++++++++++++++++++++++++++++------------------- ?1 files changed, 35 insertions(+), 20 deletions(-)drivers/mmc/mmc.c +0
-0@@ -660,7 +664,11 @@ int mmc_change_freq(struct mmc *mmc)? ? ? ?else
? ? ? ? ? ? ? ?mmc->card_caps |=MMC_MODE_HS;
Quoted 3 lines
- ? ? ? return 0;
+ exit:
+ ? ? ? err = 0;
Just initialize err to 0 at the beginning of the function, and relabel "error" as "out"
drivers/mmc/mmc.c int mmc_startup(struct mmc *mmc) · line 1138
Quoted 13 lines
+ error:
+ ? ? ?free(ext_csd);
+ ? ? ? return err;
?}?int
mmc_switch_part(int dev_num, unsigned int part_num)drivers/mmc/mmc.c +2
-0@@ -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
PATCH v2 1/1 mmc: dcache: Replace ext_csd buffer with cache aligned one
In reply to Lukasz MajewskiThis 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.
---
Changes for v2:
- Code cleanup - "out" label instead of "error"
--- drivers/mmc/mmc.c | 56 ++++++++++++++++++++++++++++++++-------------------- 1 files changed, 34 insertions(+), 22 deletions(-)
drivers/mmc/mmc.c +34 -22
@@ -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)
Signature · 1 line
1.7.2.3