Skip to contentSkip to conversation

cfi_flash.c: use addr2info

PATCH In review Ladislav Michl 5 participants 9 messages Mar 23, 2007 – May 16, 2007

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cfi_flash.c: use addr2info

Hi,

cfi_flash.c is providing functionality already present elsewhere as addr2info (in common/flash.c). Lets sava few bytes by using it.

Btw, I'm considering name flash_get_info a bit more descriptive than addr2info... I'll eventually send renaming patch on top of this one.

Use addr2info instead of private flash_get_info.

drivers/cfi_flash.c +3 -25
index 5579a1e..0536e20 100644
@@ -196,9 +196,6 @@ static int flash_write_cfiword (flash_info_t * info, ulong dest, cfiword_t cword
 static int flash_full_status_check (flash_info_t * info, flash_sect_t sector,
 				    ulong tout, char *prompt);
 ulong flash_get_size (ulong base, int banknum);
-#if defined(CFG_ENV_IS_IN_FLASH) || defined(CFG_ENV_ADDR_REDUND) || (CFG_MONITOR_BASE >= CFG_FLASH_BASE)
-static flash_info_t *flash_get_info(ulong base);
-#endif
 #ifdef CFG_FLASH_USE_BUFFER_WRITE
 static int flash_write_cfibuffer (flash_info_t * info, ulong dest, uchar * cp, int len);
 #endif
@@ -401,7 +398,7 @@ unsigned long flash_init (void)
 	flash_protect (FLAG_PROTECT_SET,
 		       CFG_MONITOR_BASE,
 		       CFG_MONITOR_BASE + monitor_flash_len  - 1,
-		       flash_get_info(CFG_MONITOR_BASE));
+		       addr2info(CFG_MONITOR_BASE));
 #endif
 
 	/* Environment protection ON by default */
@@ -409,7 +406,7 @@ unsigned long flash_init (void)
 	flash_protect (FLAG_PROTECT_SET,
 		       CFG_ENV_ADDR,
 		       CFG_ENV_ADDR + CFG_ENV_SECT_SIZE - 1,
-		       flash_get_info(CFG_ENV_ADDR));
+		       addr2info(CFG_ENV_ADDR));
 #endif
 
 	/* Redundant environment protection ON by default */
@@ -417,32 +414,13 @@ unsigned long flash_init (void)
 	flash_protect (FLAG_PROTECT_SET,
 		       CFG_ENV_ADDR_REDUND,
 		       CFG_ENV_ADDR_REDUND + CFG_ENV_SIZE_REDUND - 1,
-		       flash_get_info(CFG_ENV_ADDR_REDUND));
+		       addr2info(CFG_ENV_ADDR_REDUND));
 #endif
 	return (size);
 }
 
 /*-----------------------------------------------------------------------
  */
-#if defined(CFG_ENV_IS_IN_FLASH) || defined(CFG_ENV_ADDR_REDUND) || (CFG_MONITOR_BASE >= CFG_FLASH_BASE)
-static flash_info_t *flash_get_info(ulong base)
-{
-	int i;
-	flash_info_t * info = 0;
-
-	for (i = 0; i < CFG_MAX_FLASH_BANKS; i ++) {
-		info = & flash_info[i];
-		if (info->size && info->start[0] <= base &&
-		    base <= info->start[0] + info->size - 1)
-			break;
-	}
-
-	return i == CFG_MAX_FLASH_BANKS ? 0 : info;
-}
-#endif
-
-/*-----------------------------------------------------------------------
- */
 int flash_erase (flash_info_t * info, int s_first, int s_last)
 {
 	int rcode = 0;
#1 lore raw

[U-Boot-Users] u-boot.lds

I am trying to understand more of u-boot.lds with BUILD_DIR defined. This has to do with the fact that I need a second .lds file for a small program that needs to be run from the reset vector in both an 8241 & 8541 before u-boot starts.

I can see BUILD_DIR in u-boot/Makefile and the resulting obj := $(OBJTREE) if BUILD_DIR is defined.

My issue is that u-boot.lds works fine with BUILD_DIR defined or not. But my second .lds, call it microStart.lds works without BUILD_DIR defined, but not with BUILD_DIR defined.

It cannot find the microStart.o defined in cpu/mpc824x/microStart.o if BUILD_DIR is defined, but finds it just fine it BUILD_DIR is not defined.

I can see in u-boot that $obj is not prefixed to the linker objects and wonder how this part of automagic works so I can use the same notion with my microStart.lds.

Charles

#2 lore raw

[U-Boot-Users] u-boot.lds

Quoted 1 line · In message <[email protected]>…

I am trying to understand more of u-boot.lds with BUILD_DIR defined. This has to do with the fact that I need a second .lds file for a small program that needs to be run from the reset vector in both an 8241 & 8541 before u-boot starts.

Why don't you simply include your code in U-Boot?

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"God is a comedian playing to an audience too afraid to laugh."
- Voltaire
#3 lore raw

PATCH 1/1 cfi_flash.c: use addr2info

In reply to Ladislav Michl
Quoted 4 lines · Ladislav Michl

Hi,

cfi_flash.c is providing functionality already present elsewhere as addr2info (in common/flash.c). Lets save few bytes by using it.

Tolunay, any comments?

ladis

#4 lore raw

PATCH 1/1 cfi_flash.c: use addr2info

Quoted 7 lines · Ladislav Michl
Quoted 4 lines · Ladislav Michl

Hi,

cfi_flash.c is providing functionality already present elsewhere as addr2info (in common/flash.c). Lets save few bytes by using it.

Tolunay, any comments?

I am all for removing redundant code.

I looked at your patch when you submitted but did not carefully review yet. I feel like the function should reside inside the flash driver where it is most often used and flash driver should export it.

Tolunay

#5 lore raw

PATCH 1/1 cfi_flash.c: use addr2info

Quoted 3 lines · Tolunay Orkun

I looked at your patch when you submitted but did not carefully review yet. I feel like the function should reside inside the flash driver where it is most often used and flash driver should export it.

I disagree. This function (addr2info) is used also by board specific drivers as well as by common/cmd_load.c, common/cmd_bootm.c and common/cmd_mem.c. Therefore it should reside in common/flash.c - to be used by either board specific driver or cfi driver.

Best regards,
ladis

#6 lore raw

PATCH 1/1 cfi_flash.c: use addr2info

Quoted 5 lines · Ladislav Michl
Quoted 3 lines · Tolunay Orkun

I looked at your patch when you submitted but did not carefully review yet. I feel like the function should reside inside the flash driver where it is most often used and flash driver should export it.

Quoted 7 lines

I disagree. This function (addr2info) is used also by board specific drivers as well as by common/cmd_load.c, common/cmd_bootm.c and common/cmd_mem.c. Therefore it should reside in common/flash.c - to be used by either board specific driver or cfi driver.

Best regards,
ladis

Landis,

I was referring to the frequency of the use of the function during flash programming. I am use it is called elsewhere but when the function is in the same file compiler has more opportunities to optimize. That is why I prefer the function stay in the flash driver and exported from there.

Tolunay

#7 lore raw

PATCH 1/1 cfi_flash.c: use addr2info

Quoted 4 lines · Tolunay Orkun

I was referring to the frequency of the use of the function during flash programming. I am use it is called elsewhere but when the function is in the same file compiler has more opportunities to optimize. That is why I prefer the function stay in the flash driver and exported from there.

In drivers/cfi_flash.c flash_get_info (alias addr2info) is called only from flash_init. So what "frequency of the use" are you referring to? There is not only CFI driver, some boards have their own flash drivers. Do you want them to provide another variant of this function? Or use the one from flash.c? Why cannot cfi_flash.c use it as well? Just try to run grep -r addr2info ...

Best regards,
ladis

#8 lore raw

PATCH 1/1 cfi_flash.c: use addr2info

Quoted 11 lines · Ladislav Michl
Quoted 4 lines · Tolunay Orkun

I was referring to the frequency of the use of the function during flash programming. I am use it is called elsewhere but when the function is in the same file compiler has more opportunities to optimize. That is why I prefer the function stay in the flash driver and exported from there.

In drivers/cfi_flash.c flash_get_info (alias addr2info) is called only from flash_init. So what "frequency of the use" are you referring to? There is not only CFI driver, some boards have their own flash drivers. Do you want them to provide another variant of this function? Or use the one from flash.c? Why cannot cfi_flash.c use it as well?

Sounds reasonable to me. Just me 2 cents.

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 at denx.de
=====================================================================