[U-Boot] [PATCH 07/11] flash/cfi_flash: Use virtual sector start address, not phys

Kumar Gala galak at kernel.crashing.org
Wed Jan 14 09:38:37 CET 2009


On Dec 3, 2008, at 11:04 PM, Becky Bruce wrote:

> include/flash.h was commented to say that the address in
> flash_info->start was a physical address.  However, from u-boot's
> point of view, and looking at most flash code, it makes more
> sense for this to be a virtual address.  So I corrected the
> comment to indicate that this was a virtual address.
>
> The only flash driver that was actually treating the address
> as physical was the mtd/cfi_flash driver.  However, this code
> was using it inconsistently as it actually directly dereferenced
> the "start" element, while it used map_physmem to get a
> virtual address in other places.  I changed this driver so
> that the code which initializes the info->start field calls
> map_physmem to get a virtual address, eliminating the need for
> further map_physmem calls.  The code is now consistent.
>
> The *only* place a physical address should be used is when defining  
> the
> flash banks list that is used to initialize the flash_info struct.  I
> have fixed the one platform that was impacted by this change
> (MPC8641D).
>
> Signed-off-by: Becky Bruce <beckyb at kernel.crashing.org>
> ---
> drivers/mtd/cfi_flash.c       |   53 +++++++++++++++++ 
> +----------------------
> include/configs/MPC8641HPCN.h |    2 +-
> include/flash.h               |    2 +-
> 3 files changed, 26 insertions(+), 31 deletions(-)

Stefan,

Have you reviewed this.  I'm not sure if remvoing the map_physmem() is  
the right answer because I'm assuming Haavard added them for a reason  
(AVR32?). Should we instead change info->start to be a phys_addr_t?

- k

> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index e8afe99..292cc28 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -305,17 +305,12 @@ flash_map (flash_info_t * info, flash_sect_t  
> sect, uint offset)
> {
> 	unsigned int byte_offset = offset * info->portwidth;
>
> -	return map_physmem(info->start[sect] + byte_offset,
> -			flash_sector_size(info, sect) - byte_offset,
> -			MAP_NOCACHE);
> +	return (void *)(info->start[sect] + byte_offset);
> }
>
> static inline void flash_unmap(flash_info_t *info, flash_sect_t sect,
> 		unsigned int offset, void *addr)
> {
> -	unsigned int byte_offset = offset * info->portwidth;
> -
> -	unmap_physmem(addr, flash_sector_size(info, sect) - byte_offset);
> }
>
> / 
> *-----------------------------------------------------------------------
> @@ -793,12 +788,10 @@ static flash_sect_t find_sector (flash_info_t  
> * info, ulong addr)
> static int flash_write_cfiword (flash_info_t * info, ulong dest,
> 				cfiword_t cword)
> {
> -	void *dstaddr;
> +	void *dstaddr = (void *)dest;
> 	int flag;
> 	flash_sect_t sect;
>
> -	dstaddr = map_physmem(dest, info->portwidth, MAP_NOCACHE);
> -
> 	/* Check if Flash is (sufficiently) erased */
> 	switch (info->portwidth) {
> 	case FLASH_CFI_8BIT:
> @@ -817,10 +810,8 @@ static int flash_write_cfiword (flash_info_t *  
> info, ulong dest,
> 		flag = 0;
> 		break;
> 	}
> -	if (!flag) {
> -		unmap_physmem(dstaddr, info->portwidth);
> +	if (!flag)
> 		return ERR_NOT_ERASED;
> -	}
>
> 	/* Disable interrupts which might cause a timeout here */
> 	flag = disable_interrupts ();
> @@ -862,8 +853,6 @@ static int flash_write_cfiword (flash_info_t *  
> info, ulong dest,
> 	if (flag)
> 		enable_interrupts ();
>
> -	unmap_physmem(dstaddr, info->portwidth);
> -
> 	return flash_full_status_check (info, find_sector (info, dest),
> 					info->write_tout, "write");
> }
> @@ -877,7 +866,7 @@ static int flash_write_cfibuffer (flash_info_t *  
> info, ulong dest, uchar * cp,
> 	int cnt;
> 	int retcode;
> 	void *src = cp;
> -	void *dst = map_physmem(dest, len, MAP_NOCACHE);
> +	void *dst = dest;
> 	void *dst2 = dst;
> 	int flag = 0;
> 	uint offset = 0;
> @@ -1039,7 +1028,6 @@ static int flash_write_cfibuffer (flash_info_t  
> * info, ulong dest, uchar * cp,
> 	}
>
> out_unmap:
> -	unmap_physmem(dst, len);
> 	return retcode;
> }
> #endif /* CONFIG_SYS_FLASH_USE_BUFFER_WRITE */
> @@ -1288,7 +1276,7 @@ int write_buff (flash_info_t * info, uchar *  
> src, ulong addr, ulong cnt)
> 	/* handle unaligned start */
> 	if ((aln = addr - wp) != 0) {
> 		cword.l = 0;
> -		p = map_physmem(wp, info->portwidth, MAP_NOCACHE);
> +		p = (uchar *)wp;
> 		for (i = 0; i < aln; ++i)
> 			flash_add_byte (info, &cword, flash_read8(p + i));
>
> @@ -1300,7 +1288,6 @@ int write_buff (flash_info_t * info, uchar *  
> src, ulong addr, ulong cnt)
> 			flash_add_byte (info, &cword, flash_read8(p + i));
>
> 		rc = flash_write_cfiword (info, wp, cword);
> -		unmap_physmem(p, info->portwidth);
> 		if (rc != 0)
> 			return rc;
>
> @@ -1359,14 +1346,13 @@ int write_buff (flash_info_t * info, uchar *  
> src, ulong addr, ulong cnt)
> 	 * handle unaligned tail bytes
> 	 */
> 	cword.l = 0;
> -	p = map_physmem(wp, info->portwidth, MAP_NOCACHE);
> +	p = (uchar *)wp;
> 	for (i = 0; (i < info->portwidth) && (cnt > 0); ++i) {
> 		flash_add_byte (info, &cword, *src++);
> 		--cnt;
> 	}
> 	for (; i < info->portwidth; ++i)
> 		flash_add_byte (info, &cword, flash_read8(p + i));
> -	unmap_physmem(p, info->portwidth);
>
> 	return flash_write_cfiword (info, wp, cword);
> }
> @@ -1605,7 +1591,7 @@ static void flash_read_jedec_ids (flash_info_t  
> * info)
>  * board_flash_get_legacy needs to fill in at least:
>  * info->portwidth, info->chipwidth and info->interface for Jedec  
> probing.
>  */
> -static int flash_detect_legacy(ulong base, int banknum)
> +static int flash_detect_legacy(phys_addr_t base, int banknum)
> {
> 	flash_info_t *info = &flash_info[banknum];
>
> @@ -1621,7 +1607,10 @@ static int flash_detect_legacy(ulong base,  
> int banknum)
>
> 			for (i = 0; i < sizeof(modes) / sizeof(modes[0]); i++) {
> 				info->vendor = modes[i];
> -				info->start[0] = base;
> +				info->start[0] =
> +					(ulong)map_physmem(base,
> +							   info->portwith,
> +							   MAP_NOCACHE);
> 				if (info->portwidth == FLASH_CFI_8BIT
> 					&& info->interface == FLASH_CFI_X8X16) {
> 					info->addr_unlock1 = 0x2AAA;
> @@ -1635,8 +1624,11 @@ static int flash_detect_legacy(ulong base,  
> int banknum)
> 						info->manufacturer_id,
> 						info->device_id,
> 						info->device_id2);
> -				if (jedec_flash_match(info, base))
> +				if (jedec_flash_match(info, info->start[0]))
> 					break;
> +				else
> +					unmap_physmem(info->start[0],
> +						      MAP_NOCACHE);
> 			}
> 		}
>
> @@ -1658,7 +1650,7 @@ static int flash_detect_legacy(ulong base, int  
> banknum)
> 	return 0; /* use CFI */
> }
> #else
> -static inline int flash_detect_legacy(ulong base, int banknum)
> +static inline int flash_detect_legacy(phys_addr_t base, int banknum)
> {
> 	return 0; /* use CFI */
> }
> @@ -1799,12 +1791,12 @@ static void flash_fixup_atmel(flash_info_t  
> *info, struct cfi_qry *qry)
>  * The following code cannot be run from FLASH!
>  *
>  */
> -ulong flash_get_size (ulong base, int banknum)
> +ulong flash_get_size (phys_addr_t base, int banknum)
> {
> 	flash_info_t *info = &flash_info[banknum];
> 	int i, j;
> 	flash_sect_t sect_cnt;
> -	unsigned long sector;
> +	phys_addr_t sector;
> 	unsigned long tmp;
> 	int size_ratio;
> 	uchar num_erase_regions;
> @@ -1820,7 +1812,7 @@ ulong flash_get_size (ulong base, int banknum)
> 	info->legacy_unlock = 0;
> #endif
>
> -	info->start[0] = base;
> +	info->start[0] = (ulong)map_physmem(base, info->portwidth,  
> MAP_NOCACHE);
>
> 	if (flash_detect_cfi (info, &qry)) {
> 		info->vendor = le16_to_cpu(qry.p_id);
> @@ -1909,7 +1901,10 @@ ulong flash_get_size (ulong base, int banknum)
> 					printf("ERROR: too many flash sectors\n");
> 					break;
> 				}
> -				info->start[sect_cnt] = sector;
> +				info->start[sect_cnt] =
> +					(ulong)map_physmem(sector,
> +							   info->portwidth,
> +							   MAP_NOCACHE);
> 				sector += (erase_region_size * size_ratio);
>
> 				/*
> @@ -1986,7 +1981,7 @@ unsigned long flash_init (void)
> 	char *s = getenv("unlock");
> #endif
>
> -#define BANK_BASE(i)	(((unsigned long  
> [CFI_MAX_FLASH_BANKS])CONFIG_SYS_FLASH_BANKS_LIST)[i])
> +#define BANK_BASE(i)	(((phys_addr_t  
> [CFI_MAX_FLASH_BANKS])CONFIG_SYS_FLASH_BANKS_LIST)[i])
>
> 	/* Init: no FLASHes known */
> 	for (i = 0; i < CONFIG_SYS_MAX_FLASH_BANKS; ++i) {
> diff --git a/include/configs/MPC8641HPCN.h b/include/configs/ 
> MPC8641HPCN.h
> index 69b4c44..c25380b 100644
> --- a/include/configs/MPC8641HPCN.h
> +++ b/include/configs/MPC8641HPCN.h
> @@ -187,7 +187,7 @@ extern unsigned long get_board_sys_clk(unsigned  
> long dummy);
> 					 | CONFIG_SYS_PHYS_ADDR_HIGH)
>
>
> -#define CONFIG_SYS_FLASH_BANKS_LIST {CONFIG_SYS_FLASH_BASE}
> +#define CONFIG_SYS_FLASH_BANKS_LIST {CONFIG_SYS_FLASH_BASE_PHYS}
>
> /* Convert an address into the right format for the BR registers */
> #ifdef CONFIG_PHYS_64BIT
> diff --git a/include/flash.h b/include/flash.h
> index 6e2981c..02c6a04 100644
> --- a/include/flash.h
> +++ b/include/flash.h
> @@ -33,7 +33,7 @@ typedef struct {
> 	ulong	size;			/* total bank size in bytes		*/
> 	ushort	sector_count;		/* number of erase units		*/
> 	ulong	flash_id;		/* combined device & manufacturer code	*/
> -	ulong	start[CONFIG_SYS_MAX_FLASH_SECT];   /* physical sector start  
> addresses */
> +	ulong	start[CONFIG_SYS_MAX_FLASH_SECT];   /* virtual sector start  
> address */
> 	uchar	protect[CONFIG_SYS_MAX_FLASH_SECT]; /* sector protection  
> status	*/
> #ifdef CONFIG_SYS_FLASH_CFI
> 	uchar	portwidth;		/* the width of the port		*/
> -- 
> 1.5.6.5
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



More information about the U-Boot mailing list