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

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

[Masahiro Yamada]

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__ */
-- 
1.8.1.2

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

[Wolfgang Denk]

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 at 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

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

[Masahiro Yamada]

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 at 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



-- 
????????? ????LSI????????
??????????
?????? ???????
????? <yamada.m@jp.panasonic.com>
?617-8520  ???????????1??
?? : 050-3783-5420
?? : 7-664-2813, ?????: 664-772

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

[Stefan Roese]

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

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

[Masahiro Yamada]

Hello Stefan


> Is this patch still needed?

Yes, I think so.


Best Regards
Masahiro Yamada

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

[Stefan Roese]

On 07/30/2013 07:36 AM, Masahiro Yamada wrote:
>> Is this patch still needed?
> 
> Yes, I think so.

Okay:

Applied to u-boot-cfi-flash.

Thanks,
Stefan