[U-Boot-Users] [PATCH] 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.
Signed-off-by: Ladislav Michl ladis@linux-mips.org
Use addr2info instead of private flash_get_info.
diff --git a/drivers/cfi_flash.c b/drivers/cfi_flash.c index 5579a1e..0536e20 100644 --- a/drivers/cfi_flash.c +++ b/drivers/cfi_flash.c @@ -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;

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

In message 9F3F0A752CAEBE4FA7E906CC2FBFF57C069B64@MERCURY.inside.istor.com you wrote:
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

Ladislav Michl wrote:
On Fri, Mar 23, 2007 at 12:52:35PM +0100, Ladislav Michl wrote:
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

On Wed, May 02, 2007 at 04:50:48PM -0500, Tolunay Orkun wrote:
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

Ladislav Michl wrote:
On Wed, May 02, 2007 at 04:50:48PM -0500, Tolunay Orkun wrote:
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
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

On Thu, May 03, 2007 at 04:06:16PM -0500, Tolunay Orkun wrote:
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

On Wednesday 16 May 2007, Ladislav Michl wrote:
On Thu, May 03, 2007 at 04:06:16PM -0500, Tolunay Orkun wrote:
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@denx.de =====================================================================
participants (5)
-
Charles Krinke
-
Ladislav Michl
-
Stefan Roese
-
Tolunay Orkun
-
Wolfgang Denk