[U-Boot] [PATCH] cfi_flash: Add prototypes of overridable functions

This commit adds some prototypes into include/mtd/cfi_flash.h. These functions are defined with a weak attribute in drivers/mtd/cfi_flash.c. This means they can be overrided by board-specific ones if necessary.
When defining such functions under board/ directory or somewhere, cfi_flash.h should be included. This makes sure that board-specfic cfi functions are defined in a correct prototype.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com --- include/mtd/cfi_flash.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h index b644b91..f97cf66 100644 --- a/include/mtd/cfi_flash.h +++ b/include/mtd/cfi_flash.h @@ -184,5 +184,19 @@ extern int cfi_flash_num_flash_banks;
void flash_write_cmd(flash_info_t * info, flash_sect_t sect, uint offset, u32 cmd); +phys_addr_t cfi_flash_bank_addr(int i); +unsigned long cfi_flash_bank_size(int i); +void flash_cmd_reset(flash_info_t *info); + +#ifdef CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS +void flash_write8(u8 value, void *addr); +void flash_write16(u16 value, void *addr); +void flash_write32(u32 value, void *addr); +void flash_write64(u64 value, void *addr); +u8 flash_read8(void *addr); +u16 flash_read16(void *addr); +u32 flash_read32(void *addr); +u64 flash_read64(void *addr); +#endif
#endif /* __CFI_FLASH_H__ */

Dear Masahiro Yamada,
In message 1370505244-5015-1-git-send-email-yamada.m@jp.panasonic.com you wrote:
This commit adds some prototypes into include/mtd/cfi_flash.h. These functions are defined with a weak attribute in drivers/mtd/cfi_flash.c. This means they can be overrided by board-specific ones if necessary.
When defining such functions under board/ directory or somewhere, cfi_flash.h should be included. This makes sure that board-specfic cfi functions are defined in a correct prototype.
As is, this would just add dead code. Please resubmit this with any patch that would actually make use of this feature. this would also allow us to understand why this approch is needed at all.
+#ifdef CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS
When resubmitting, please keep in mind that new CONFIG_* options like this must be documented in the README.
Best regards,
Wolfgang Denk

Hello, Wolfgang Denk.
As is, this would just add dead code. Please resubmit this with any patch that would actually make use of this feature. this would also allow us to understand why this approch is needed at all.
I'm not sure this is the best approach, but these overrides are actually used by some boards.
I don't know very much about other companies, but I can explain how this override is being used for our boards in my company. (Our board is not public. Only used inside Panasonic for LSI debug. The board specific code is not posted yet.) Our boards have very flexible memory slots. We can use both SRAM and NOR flash for this slots and change base address easily. So we probe NOR flash and configure base address at boot time, not compile time. For this purpose, we override cfi_flash_bank_addr function with our own.
When I see board/lwmon5/lwmon5.c, this feature seems to be used for similar purpose.
+#ifdef CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS
When resubmitting, please keep in mind that new CONFIG_* options like this must be documented in the README.
CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS seems to be introduced by the following commit. I added Stefan in CC.
commit 45aa5a7f4d5bcb79927ddfc896c1d7c4326e235d Author: Stefan Roese sr@denx.de Date: Mon Nov 17 14:45:22 2008 +0100
cfi_flash: Make all flash access functions weak
This patch defines all flash access functions as weak so that they can be overridden by board specific versions.
This will be used by the upcoming VCTH board support where the NOR FLASH unfortunately can't be accessed memory-mapped. Special accessor functions are needed here.
To enable this weak functions you need to define CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS in your board config header. Otherwise the "old" default functions will be used resulting in smaller code.
Signed-off-by: Stefan Roese sr@denx.de Acked-by: Haavard Skinnemoen haavard.skinnemoen@atmel.com
Best Regards Masahiro Yamada
On Thu, 06 Jun 2013 12:02:56 +0200 Wolfgang Denk wd@denx.de wrote:
Dear Masahiro Yamada,
In message 1370505244-5015-1-git-send-email-yamada.m@jp.panasonic.com you wrote:
This commit adds some prototypes into include/mtd/cfi_flash.h. These functions are defined with a weak attribute in drivers/mtd/cfi_flash.c. This means they can be overrided by board-specific ones if necessary.
When defining such functions under board/ directory or somewhere, cfi_flash.h should be included. This makes sure that board-specfic cfi functions are defined in a correct prototype.
As is, this would just add dead code. Please resubmit this with any patch that would actually make use of this feature. this would also allow us to understand why this approch is needed at all.
+#ifdef CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS
When resubmitting, please keep in mind that new CONFIG_* options like this must be documented in the README.
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: wd@denx.de Here is an Appalachian version of management's answer to those who are concerned with the fate of the project: "Don't worry about the mule. Just load the wagon." - Mike Dennison's hillbilly uncle

On 06/06/2013 09:54 AM, Masahiro Yamada wrote:
This commit adds some prototypes into include/mtd/cfi_flash.h. These functions are defined with a weak attribute in drivers/mtd/cfi_flash.c. This means they can be overrided by board-specific ones if necessary.
When defining such functions under board/ directory or somewhere, cfi_flash.h should be included. This makes sure that board-specfic cfi functions are defined in a correct prototype.
Signed-off-by: Masahiro Yamada yamada.m@jp.panasonic.com
Is this patch still needed?
Thanks, Stefan
participants (3)
-
Masahiro Yamada
-
Stefan Roese
-
Wolfgang Denk