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 =====================================================================