[U-Boot] [PATCH v2] 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 --- v2: - Added configuration option description to the README - Fix bug in NOR FLASH end detection in env_flash.c (spotted by Wolfgang) - Further "cleanup" of the env_flash code supporting non memory-mapped NOR FLASH. - Macros in crc32.h now wrapped in "do { ... } while (0)" as suggested by Wolfgang
README | 17 ++++++++++++ common/cmd_mem.c | 66 +++++++++++++++++++++++++++++++++++++----------- common/env_flash.c | 70 +++++++++++++++++++++++++++++++++++++++++--------- lib_generic/crc32.c | 27 ++++++++++++++++--- 4 files changed, 148 insertions(+), 32 deletions(-)
diff --git a/README b/README index 861ea83..0fd07a9 100644 --- a/README +++ b/README @@ -2180,6 +2180,23 @@ Configuration Settings: digits and dots. Recommended value: 45 (9..1) for 80 column displays, 15 (3..1) for 40 column displays.
+- CONFIG_FLASH_NOT_MEM_MAPPED + Unfortunately there are boards, where the NOR FLASH can't + be accessed memory mapped. So we needed to find a way to + access those FLASH's from the CFI driver and the environment. + 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. So we need + to additionally define the following options to configure + the NOR FLASH range: + + - CONFIG_FLASH_BASE + Starting address of the NOR FLASH area + + - CONFIG_FLASH_END + Highest possible address of the NOR FLASH area + - CONFIG_SYS_RX_ETH_BUFFER: Defines the number of Ethernet receive buffers. On some Ethernet controllers it is recommended to set this value 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..ee7c4d7 100644 --- a/common/env_flash.c +++ b/common/env_flash.c @@ -85,9 +85,50 @@ 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); + +#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 */ + +/* + * On boards where the NOR FLASH is not memory mapped, this function + * returns 1 when the address is in the NOR FLASH range. On "normal" boards, + * where the NOR FLASH is memory mapped this function always returns 0. + * This way GCC removes the true path in the following functions calling + * addr_in_flash_range() by optimization. + */ +static inline int addr_in_flash_range(u32 addr) +{ +#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) + if ((addr >= CONFIG_FLASH_BASE) && (addr <= CONFIG_FLASH_END)) + return 1; +#endif + return 0; +} + +static inline u8 read_env8(u8 *addr) +{ + if (addr_in_flash_range((u32)addr)) + return flash_read8((void *)addr); + return *addr; +} + +static inline u32 read_env32(u32 *addr) +{ + if (addr_in_flash_range((u32)addr)) + return flash_read32((void *)addr); + return *addr; +}
uchar env_get_char_spec (int index) { + if (addr_in_flash_range((u32)gd->env_addr)) + return (flash_read8((void *)gd->env_addr + index)); return ( *((uchar *)(gd->env_addr + index)) ); }
@@ -97,15 +138,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 +212,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 +290,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 +326,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 +390,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 +403,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 +420,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..51242d7 100644 --- a/lib_generic/crc32.c +++ b/lib_generic/crc32.c @@ -148,10 +148,29 @@ const uint32_t * ZEXPORT get_crc_table() #endif
/* ========================================================================= */ -#define DO1(buf) crc = crc_table[((int)crc ^ (*buf++)) & 0xff] ^ (crc >> 8); -#define DO2(buf) DO1(buf); DO1(buf); -#define DO4(buf) DO2(buf); DO2(buf); -#define DO8(buf) DO4(buf); DO4(buf); +#if defined(CONFIG_FLASH_NOT_MEM_MAPPED) +u8 flash_read8(void *addr); +#define DO1(buf) \ + do { \ + 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); \ + } \ + } while (0) +#else +#define DO1(buf) \ + do { \ + crc = crc_table[((int)crc ^ (*buf++)) & 0xff] ^ (crc >> 8); \ + } while (0) +#endif +#define DO2(buf) do { DO1(buf); DO1(buf); } while (0) +#define DO4(buf) do { DO2(buf); DO2(buf); } while (0) +#define DO8(buf) do { DO4(buf); DO4(buf); } while (0)
/* ========================================================================= */ uint32_t ZEXPORT crc32 (uint32_t crc, const Bytef *buf, uInt len)

On Wed, Dec 10, 2008 at 11:41:11AM +0100, Stefan Roese wrote:
+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)
__const__? The other underscores seem gratuitous as well.
+{ +#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 +}
What if a board has dataflash *and* NOR flash?
diff --git a/common/env_flash.c b/common/env_flash.c index 75ee8dd..ee7c4d7 100644 --- a/common/env_flash.c +++ b/common/env_flash.c @@ -85,9 +85,50 @@ 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);
Should go in a header file.
+#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 */
No semicolon after memcpy, or FLASH_READ_MEMCPY won't behave like a function call in certain contexts (if/else, for loops, etc).
+#define DO1(buf) \
- do { \
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); \
} \
- } while (0)
Maybe make this an inline function?
-Scott

On Wednesday 10 December 2008, Scott Wood wrote:
On Wed, Dec 10, 2008 at 11:41:11AM +0100, Stefan Roese wrote:
+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)
__const__? The other underscores seem gratuitous as well.
Not sure where I copied this from. I'll remove the underscores in the next version.
+{ +#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 +}
What if a board has dataflash *and* NOR flash?
You probably mean a board with dataflash *and* NOR FLASH that can't be accessed memory-mapped. We currently don't have such a board. The upcoming VCT board port is the first with such an (cumbersome) local bus interface. And I hope it will be the last.
diff --git a/common/env_flash.c b/common/env_flash.c index 75ee8dd..ee7c4d7 100644 --- a/common/env_flash.c +++ b/common/env_flash.c @@ -85,9 +85,50 @@ 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);
Should go in a header file.
OK.
+#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 */
No semicolon after memcpy, or FLASH_READ_MEMCPY won't behave like a function call in certain contexts (if/else, for loops, etc).
Right. Thanks for spotting.
+#define DO1(buf) \
- do { \
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); \
} \
- } while (0)
Maybe make this an inline function?
That would be a bigger change since this macro changes "buf" and "crc" without having those parameters passed to it. I didn't want to make this bigger change if not really necessary.
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)
-
Scott Wood
-
Stefan Roese