Dear Stefan Roese, Sorry for the late review...
In message <[EMAIL PROTECTED]> 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 -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [EMAIL PROTECTED] There is nothing in this world constant but inconstancy. - Swift _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot