[U-Boot] [PATCH] Add support for boards where the NOR FLASH is not memory-mapped
This patch adds the CONFIG_FLASH_NOT_MEM_MAPPED define which can be used on boards where the NOR FLASH is not memory-mapped and special accessor functions are needed to access the NOR FLASH. This affects the memory commands (cmd_mem.c) and the environment (env_flash.c).
Boards using CONFIG_FLASH_NOT_MEM_MAPPED need to additionally specify CONFIG_FLASH_BASE and CONFIG_FLASH_END so that the NOR FLASH region can be detected.
This will be used by the upcoming VCTH board support.
Signed-off-by: Stefan Roese sr@denx.de --- common/cmd_mem.c | 66 +++++++++++++++++++++++++++++++++++++++----------- common/env_flash.c | 61 +++++++++++++++++++++++++++++++++++++---------- lib_generic/crc32.c | 11 ++++++++ 3 files changed, 110 insertions(+), 28 deletions(-)
diff --git a/common/cmd_mem.c b/common/cmd_mem.c index d7666c2..f545f6a 100644 --- a/common/cmd_mem.c +++ b/common/cmd_mem.c @@ -43,6 +43,43 @@ #define PRINTF(fmt,args...) #endif
+#if defined(CONFIG_HAS_DATAFLASH) +#define CONFIG_FLASH_NOT_MEM_MAPPED +#define OK DATAFLASH_OK +#endif + +#if !defined(OK) +#define OK 1 +#endif + +void *board_flash_read_memcpy(void *__to, __const__ void *__from, int n); + +#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) +static inline int flash_read(void *__to, __const__ void *__from, int n) +{ +#if defined(CONFIG_HAS_DATAFLASH) + return read_dataflash(__from, n, __to); +#elif defined(CONFIG_FLASH_NOT_MEM_MAPPED) + if ((addr2info((u32)__from) != NULL)) + board_flash_read_memcpy(__to, __from, n); + else + memcpy(__to, __from, n); + return OK; +#endif +} +#endif /* defined(CONFIG_FLASH_NOT_MEM_MAPPED) */ + +static inline int is_flash_addr(u32 addr) +{ +#if defined(CONFIG_HAS_DATAFLASH) + return addr_dataflash(addr); +#elif defined(CONFIG_FLASH_NOT_MEM_MAPPED) + return (addr2info(addr) != NULL); +#else + return 1; +#endif +} + static int mod_mem(cmd_tbl_t *, int, int, int, char *[]);
/* Display values from last command. @@ -63,7 +100,7 @@ static ulong base_address = 0; int do_mem_md ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) { ulong addr, length; -#if defined(CONFIG_HAS_DATAFLASH) +#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) ulong nbytes, linebytes; #endif int size; @@ -100,7 +137,7 @@ int do_mem_md ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) length = simple_strtoul(argv[2], NULL, 16); }
-#if defined(CONFIG_HAS_DATAFLASH) +#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) /* Print the lines. * * We buffer all read data, so we can make sure data is read only @@ -112,10 +149,13 @@ int do_mem_md ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) void* p; linebytes = (nbytes>DISP_LINE_LEN)?DISP_LINE_LEN:nbytes;
- rc = read_dataflash(addr, (linebytes/size)*size, linebuf); - p = (rc == DATAFLASH_OK) ? linebuf : (void*)addr; + rc = flash_read(linebuf, (void *)addr, (linebytes/size)*size); + p = (rc == OK) ? linebuf : (void *)addr; print_buffer(addr, p, size, linebytes/size, DISP_LINE_LEN/size);
+ /* Clear rc, otherwise command repeat doesn't work */ + rc = 0; + nbytes -= linebytes; addr += linebytes; if (ctrlc()) { @@ -294,9 +334,9 @@ int do_mem_cmp (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
count = simple_strtoul(argv[3], NULL, 16);
-#ifdef CONFIG_HAS_DATAFLASH - if (addr_dataflash(addr1) | addr_dataflash(addr2)){ - puts ("Comparison with DataFlash space not supported.\n\r"); +#ifdef CONFIG_FLASH_NOT_MEM_MAPPED + if (is_flash_addr(addr1) || is_flash_addr(addr2)) { + puts ("Comparison with FLASH space not supported.\n\r"); return 0; } #endif @@ -385,11 +425,7 @@ int do_mem_cp ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
#ifndef CONFIG_SYS_NO_FLASH /* check if we are copying to Flash */ - if ( (addr2info(dest) != NULL) -#ifdef CONFIG_HAS_DATAFLASH - && (!addr_dataflash(dest)) -#endif - ) { + if (is_flash_addr(dest)) { int rc;
puts ("Copy to Flash... "); @@ -1010,9 +1046,9 @@ mod_mem(cmd_tbl_t *cmdtp, int incrflag, int flag, int argc, char *argv[]) addr += base_address; }
-#ifdef CONFIG_HAS_DATAFLASH - if (addr_dataflash(addr)){ - puts ("Can't modify DataFlash in place. Use cp instead.\n\r"); +#ifdef CONFIG_FLASH_NOT_MEM_MAPPED + if (is_flash_addr(addr)) { + puts ("Can't modify FLASH in place. Use cp instead.\n\r"); return 0; } #endif diff --git a/common/env_flash.c b/common/env_flash.c index 75ee8dd..9c53ec0 100644 --- a/common/env_flash.c +++ b/common/env_flash.c @@ -85,9 +85,41 @@ static ulong end_addr_new = CONFIG_ENV_ADDR_REDUND + CONFIG_ENV_SECT_SIZE - 1; extern uchar default_environment[]; extern int default_environment_size;
+void *board_flash_read_memcpy(void *__to, __const__ void *__from, int n); +u8 flash_read8(void *addr); +u32 flash_read32(void *addr); + +static inline u8 read_env8(u8 *addr) +{ +#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) + if (((u32)addr >= CONFIG_FLASH_BASE) && ((u32)addr < CONFIG_FLASH_END)) + return flash_read8((void *)addr); +#endif + return *addr; +} + +static inline u32 read_env32(u32 *addr) +{ +#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) + if (((u32)addr >= CONFIG_FLASH_BASE) && ((u32)addr < CONFIG_FLASH_END)) + return flash_read32((void *)addr); +#endif + return *addr; +} + +#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) +#define FLASH_READ_MEMCPY(d, s, n) board_flash_read_memcpy(d, s, n) +#else +#define FLASH_READ_MEMCPY(d, s, n) memcpy(d, s, n); +#endif /* CONFIG_FLASH_NOT_MEM_MAPPED */
uchar env_get_char_spec (int index) { +#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) + if (((u32)gd->env_addr >= CONFIG_FLASH_BASE) && + ((u32)gd->env_addr < CONFIG_FLASH_END)) + return (flash_read8((void *)gd->env_addr + index)); +#endif return ( *((uchar *)(gd->env_addr + index)) ); }
@@ -97,15 +129,17 @@ int env_init(void) { int crc1_ok = 0, crc2_ok = 0;
- uchar flag1 = flash_addr->flags; - uchar flag2 = flash_addr_new->flags; + uchar flag1 = read_env8(&flash_addr->flags); + uchar flag2 = read_env8(&flash_addr_new->flags);
ulong addr_default = (ulong)&default_environment[0]; ulong addr1 = (ulong)&(flash_addr->data); ulong addr2 = (ulong)&(flash_addr_new->data);
- crc1_ok = (crc32(0, flash_addr->data, ENV_SIZE) == flash_addr->crc); - crc2_ok = (crc32(0, flash_addr_new->data, ENV_SIZE) == flash_addr_new->crc); + crc1_ok = (crc32(0, flash_addr->data, ENV_SIZE) == + read_env32(&flash_addr->crc)); + crc2_ok = (crc32(0, flash_addr_new->data, ENV_SIZE) == + read_env32(&flash_addr_new->crc));
if (crc1_ok && ! crc2_ok) { gd->env_addr = addr1; @@ -169,8 +203,9 @@ int saveenv(void) up_data); goto Done; } - memcpy(saved_data, - (void *)((long)flash_addr_new + CONFIG_ENV_SIZE), up_data); + FLASH_READ_MEMCPY(saved_data, + (void *)((long)flash_addr_new + CONFIG_ENV_SIZE), + up_data); debug ("Data (start 0x%x, len 0x%x) saved at 0x%x\n", (long)flash_addr_new + CONFIG_ENV_SIZE, up_data, saved_data); @@ -246,7 +281,7 @@ Done:
int env_init(void) { - if (crc32(0, env_ptr->data, ENV_SIZE) == env_ptr->crc) { + if (crc32(0, env_ptr->data, ENV_SIZE) == read_env32(&env_ptr->crc)) { gd->env_addr = (ulong)&(env_ptr->data); gd->env_valid = 1; return(0); @@ -282,7 +317,7 @@ int saveenv(void) flash_sect_addr, (ulong)flash_addr, flash_offset);
/* copy old contents to temporary buffer */ - memcpy (env_buffer, (void *)flash_sect_addr, CONFIG_ENV_SECT_SIZE); + FLASH_READ_MEMCPY(env_buffer, (void *)flash_sect_addr, CONFIG_ENV_SECT_SIZE);
/* copy current environment to temporary buffer */ memcpy ((uchar *)((unsigned long)env_buffer + flash_offset), @@ -346,9 +381,9 @@ void env_relocate_spec (void) end_addr_new = ltmp; }
- if (flash_addr_new->flags != OBSOLETE_FLAG && + if (read_env8(&flash_addr_new->flags) != OBSOLETE_FLAG && crc32(0, flash_addr_new->data, ENV_SIZE) == - flash_addr_new->crc) { + read_env32(&flash_addr_new->crc)) { char flag = OBSOLETE_FLAG;
gd->env_valid = 2; @@ -359,8 +394,8 @@ void env_relocate_spec (void) flash_sect_protect (1, (ulong)flash_addr_new, end_addr_new); }
- if (flash_addr->flags != ACTIVE_FLAG && - (flash_addr->flags & ACTIVE_FLAG) == ACTIVE_FLAG) { + if (read_env8(&flash_addr->flags) != ACTIVE_FLAG && + (read_env8(&flash_addr->flags) & ACTIVE_FLAG) == ACTIVE_FLAG) { char flag = ACTIVE_FLAG;
gd->env_valid = 2; @@ -376,7 +411,7 @@ void env_relocate_spec (void) "reading environment; recovered successfully\n\n"); #endif /* CONFIG_ENV_ADDR_REDUND */ #ifdef CMD_SAVEENV - memcpy (env_ptr, (void*)flash_addr, CONFIG_ENV_SIZE); + FLASH_READ_MEMCPY(env_ptr, (void*)flash_addr, CONFIG_ENV_SIZE); #endif #endif /* ! ENV_IS_EMBEDDED || CONFIG_ENV_ADDR_REDUND */ } diff --git a/lib_generic/crc32.c b/lib_generic/crc32.c index b6a7a91..d698cc6 100644 --- a/lib_generic/crc32.c +++ b/lib_generic/crc32.c @@ -148,7 +148,18 @@ const uint32_t * ZEXPORT get_crc_table() #endif
/* ========================================================================= */ +#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) +u8 flash_read8(void *addr); +#define DO1(buf) \ + if (((u32)buf >= CONFIG_FLASH_BASE) && ((u32)buf < CONFIG_FLASH_END)) { \ + crc = crc_table[((int)crc ^ (flash_read8((void *)buf++))) & 0xff] ^ \ + (crc >> 8); \ + } else { \ + crc = crc_table[((int)crc ^ (*buf++)) & 0xff] ^ (crc >> 8); \ + } +#else #define DO1(buf) crc = crc_table[((int)crc ^ (*buf++)) & 0xff] ^ (crc >> 8); +#endif #define DO2(buf) DO1(buf); DO1(buf); #define DO4(buf) DO2(buf); DO2(buf); #define DO8(buf) DO4(buf); DO4(buf);
Dear Stefan Roese,
Sorry for the late review...
In message 1226493138-28470-1-git-send-email-sr@denx.de you wrote:
This patch adds the CONFIG_FLASH_NOT_MEM_MAPPED define which can be used on boards where the NOR FLASH is not memory-mapped and special accessor functions are needed to access the NOR FLASH. This affects the memory commands (cmd_mem.c) and the environment (env_flash.c).
Boards using CONFIG_FLASH_NOT_MEM_MAPPED need to additionally specify CONFIG_FLASH_BASE and CONFIG_FLASH_END so that the NOR FLASH region can be detected.
You have to document this (at least in the README).
But there is a general problem with this approach: U-Boot is based on the design that the actual flash size is auto-detected, i. e. it is always assumed to be unknown at compile time. Therefore it is impossible to set something like CONFIG_FLASH_END at compile time.
+#if defined(CONFIG_FLASH_NOT_MEM_MAPPED)
- if (((u32)addr >= CONFIG_FLASH_BASE) && ((u32)addr < CONFIG_FLASH_END))
And this looks as if CONFIG_FLASH_END was not the "FLASH_END" address, i. e. the last address in flash, but "FLASH_END" + 1 ?
+#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) +#define FLASH_READ_MEMCPY(d, s, n) board_flash_read_memcpy(d, s, n) +#else +#define FLASH_READ_MEMCPY(d, s, n) memcpy(d, s, n); +#endif /* CONFIG_FLASH_NOT_MEM_MAPPED */
This is really, really ugly - and error prone, as you must be extremely careful which of the functions you may call might use memcpy() or similar internally.
You know that I know of the specific problems of this hardware, but anyway - I really dislike having to add such code.
+#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) +u8 flash_read8(void *addr); +#define DO1(buf) \
- if (((u32)buf >= CONFIG_FLASH_BASE) && ((u32)buf < CONFIG_FLASH_END)) { \
crc = crc_table[((int)crc ^ (flash_read8((void *)buf++))) & 0xff] ^ \(crc >> 8); \- } else { \
crc = crc_table[((int)crc ^ (*buf++)) & 0xff] ^ (crc >> 8); \- }
+#else #define DO1(buf) crc = crc_table[((int)crc ^ (*buf++)) & 0xff] ^ (crc >> 8); +#endif #define DO2(buf) DO1(buf); DO1(buf); #define DO4(buf) DO2(buf); DO2(buf); #define DO8(buf) DO4(buf); DO4(buf);
Please wrap all such macros in the usual "do { ... } while (0)" wrappers.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Wednesday 10 December 2008, Wolfgang Denk wrote:
Sorry for the late review...
In message 1226493138-28470-1-git-send-email-sr@denx.de you wrote:
This patch adds the CONFIG_FLASH_NOT_MEM_MAPPED define which can be used on boards where the NOR FLASH is not memory-mapped and special accessor functions are needed to access the NOR FLASH. This affects the memory commands (cmd_mem.c) and the environment (env_flash.c).
Boards using CONFIG_FLASH_NOT_MEM_MAPPED need to additionally specify CONFIG_FLASH_BASE and CONFIG_FLASH_END so that the NOR FLASH region can be detected.
You have to document this (at least in the README).
No, I missed it. I'll send an updated patch later this week.
But there is a general problem with this approach: U-Boot is based on the design that the actual flash size is auto-detected, i. e. it is always assumed to be unknown at compile time. Therefore it is impossible to set something like CONFIG_FLASH_END at compile time.
Yes, this approach is not optimal but I found no other way to work with the CFI driver and especially the environment in NOR without such an "extension". But CONFIG_FLASH_END doesn't have point to the real end of the NOR FLASH but the highest possible address. So auto-detection should still be possible in such an area. Here an example from the VCTH board port:
/* * For the non-memory-mapped NOR FLASH, we need to define the * NOR FLASH area. This can't be detected via the addr2info() * function, since we check for flash access in the very early * U-Boot code, before the NOR FLASH is detected. */ #define CONFIG_FLASH_BASE 0xb0000000 #define CONFIG_FLASH_END 0xbfffffff
So the FLASH size could be a maximum of 256MB in this case. Not optimal but at least not totally fixed.
+#if defined(CONFIG_FLASH_NOT_MEM_MAPPED)
- if (((u32)addr >= CONFIG_FLASH_BASE) && ((u32)addr < CONFIG_FLASH_END))
And this looks as if CONFIG_FLASH_END was not the "FLASH_END" address, i. e. the last address in flash, but "FLASH_END" + 1 ?
Good catch. I'll fix this to "((u32)addr <= CONFIG_FLASH_END).
+#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) +#define FLASH_READ_MEMCPY(d, s, n) board_flash_read_memcpy(d, s, n) +#else +#define FLASH_READ_MEMCPY(d, s, n) memcpy(d, s, n); +#endif /* CONFIG_FLASH_NOT_MEM_MAPPED */
This is really, really ugly - and error prone, as you must be extremely careful which of the functions you may call might use memcpy() or similar internally.
You know that I know of the specific problems of this hardware, but anyway - I really dislike having to add such code.
Yes, we agree that this is not a "nice" solution. But this is the best I could come up after several approaches. I'm open to better suggestions.
+#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) +u8 flash_read8(void *addr); +#define DO1(buf) \
- if (((u32)buf >= CONFIG_FLASH_BASE) && ((u32)buf < CONFIG_FLASH_END)) {
\ + crc = crc_table[((int)crc ^ (flash_read8((void *)buf++))) & 0xff] ^ \ + (crc >> 8); \
- } else { \
crc = crc_table[((int)crc ^ (*buf++)) & 0xff] ^ (crc >> 8); \- }
+#else #define DO1(buf) crc = crc_table[((int)crc ^ (*buf++)) & 0xff] ^ (crc >> 8); +#endif #define DO2(buf) DO1(buf); DO1(buf); #define DO4(buf) DO2(buf); DO2(buf); #define DO8(buf) DO4(buf); DO4(buf);
Please wrap all such macros in the usual "do { ... } while (0)" wrappers.
OK, will do in the next version.
Thanks for the review.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
participants (2)
-
Stefan Roese -
Wolfgang Denk