[U-Boot] [PATCH v3] Add support for boards where the NOR FLASH is not memory-mapped

Ulf Samuelsson ulf.samuelsson at atmel.com
Fri Jan 16 08:32:05 CET 2009


tor 2008-12-11 klockan 12:11 +0100 skrev Stefan Roese:
> 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.
> 

If I interpret Wolfgang and others correctly,
NOR FLASH which does not support random access is STORAGE
and should not be accessible using memory commands,
because it makes the commands harder to maintain.

Logically, You should therefore define an "nf" command or similar
which accesses the memory in the same way as other "storage" do.

Personally, I think it is MUCH cleaner to keep the memory
command and just call a function 

	srcdscr = translate(srcaddr);
	dstdscr = translate(dstaddr);
	while(requested_size > 0) {
		copied = memcpydscr(srcdscr,dstdscr, requested_size);
		if((copied == 0) && (requested_size > 0) {
			// Error handling
		}
		requested_size -= copied;
	}

Each memory technology could register a translation driver
and will parse the argument and decide if it is
the "owner" of this argument, and if so, it will 
return a pointer to a descriptor.
If not, the argument is passed on to the next
translation driver until caught.

During the copy, the src memory driver will read sectors
and cache them allowing the dst memory driver
to treat the src memory as RAM.
At the end of the command, any cache contents are invalidated.

The advantage of this, is that once the memory driver is 
written it will be fairly easy to support 
new memory technology without having to change the
commands themselves.

I know it was shot down 1-2 years, ago but so were other
ideas which now has been implemented in U-boot,
so I think we should revisit this.

If extending the memory commands this way, is allowed
after NAKing future extensions for dataflash,
then the decision process is clearly flawed.


BR
Ulf Samuelsson


> This will be used by the upcoming VCT board support.
> 
> Signed-off-by: Stefan Roese <sr at denx.de>
> ---
> v3:
> - Removed some gratuitous underscores in function parameters
> - Prototypes moved to header (common.h)
> - Removed ";" after macro definition
> 
> 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    |   64 +++++++++++++++++++++++++++++++++++++-----------
>  common/env_flash.c  |   66 +++++++++++++++++++++++++++++++++++++++++----------
>  include/common.h    |    8 ++++++
>  lib_generic/crc32.c |   26 +++++++++++++++++---
>  5 files changed, 149 insertions(+), 32 deletions(-)
> 
> diff --git a/README b/README
> index 2a553c2..3414d66 100644
> --- a/README
> +++ b/README
> @@ -2198,6 +2198,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..29eda3e 100644
> --- a/common/cmd_mem.c
> +++ b/common/cmd_mem.c
> @@ -43,6 +43,41 @@
>  #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
> +
> +#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 +98,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 +135,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 +147,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 +332,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 +423,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 +1044,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..becc0dc 100644
> --- a/common/env_flash.c
> +++ b/common/env_flash.c
> @@ -85,9 +85,46 @@ static ulong end_addr_new = CONFIG_ENV_ADDR_REDUND + CONFIG_ENV_SECT_SIZE - 1;
>  extern uchar default_environment[];
>  extern int default_environment_size;
>  
> +#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 +134,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 +208,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 +286,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 +322,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 +386,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 +399,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 +416,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/include/common.h b/include/common.h
> index 5968036..18687c6 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -716,6 +716,14 @@ int cpu_reset(int nr);
>  int cpu_release(int nr, int argc, char *argv[]);
>  #endif
>  
> +/*
> + * Functions only needed when CONFIG_FLASH_NOT_MEM_MAPPED is defined
> + * for boards where the NOR FLASH can't be accessed memory-mapped.
> + */
> +void *board_flash_read_memcpy(void *to, const void *from, size_t n);
> +u8 flash_read8(void *addr);
> +u32 flash_read32(void *addr);
> +
>  #ifdef CONFIG_POST
>  #define CONFIG_HAS_POST
>  #endif
> diff --git a/lib_generic/crc32.c b/lib_generic/crc32.c
> index b6a7a91..9067209 100644
> --- a/lib_generic/crc32.c
> +++ b/lib_generic/crc32.c
> @@ -148,10 +148,28 @@ 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)
> +#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)
-- 
Ulf Samuelsson <ulf.samuelsson at atmel.com>
Atmel Nordic AB



More information about the U-Boot mailing list