Hi Frank,
On Tuesday 31 May 2011 10:35:17 Frank Svendsbøe wrote:
> We have a board that feature NOR flash and hardware write protection
> is handled by controlling the write enable pin. When write protection
> is enabled, the nWE pin is forced high by external logic. The memory
> controller and/or CFI logic is unaware of this, and since CFI uses
> write enable as part of the CFI command set, a CFI probing will fail
> when write protection is enabled.
>
> The rationale for controlling nWE and not WP (write protection) is
> that WP only protects the first sector of the device. In our case this
> is less than the size of the bootloader (U-boot), since that occupies
> two sectors. Due to this the built-in NOR write protection is rather
> useless.
Understood. But why don't you disable write-protection when you first call
flash_init()? And enable the write-protection after the chip is correctly
detected?
> Our current solution based on controlling nWE is to hardcode flash
> geometry in board code when flash protection is enabled. In order to
> use CFI as intended when write protection is disabled, we call the
> generic flash_init function as defined in
> drivers/mtd/cfi_flash.c.
How is write-protection enabled/disabled on your board?
> When protection is enabled we hardcode the
> geometry info in board code. In order separate our flash_init and the
> generic flash_init, and be able to call both, we've introduced a new
> ifdef to cfi_flash.c called CONFIG_CFI_FLASH_OVERRIDE_INIT. Like
> this:
>
> ----
>
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index 6039e1f..772096e 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -176,6 +176,10 @@ u64 flash_read64(void *addr)__attribute__((weak,
> alias("__flash_read64")));
> #define flash_read64 __flash_read64
> #endif
>
> +#ifdef CONFIG_CFI_FLASH_OVERRIDE_INIT
> +#define flash_init __flash_init
> +#endif
> +
> /*-----------------------------------------------------------------------
> */
> #if defined(CONFIG_ENV_IS_IN_FLASH) ||
> defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >=
>
> ----
>
> Now, in board code our redefined flash_init will be called. But if
> write protection is off, we call the original function,
> eg. __flash_init.
>
> Using the preprocessor is often considered bad design. However, the
> alternatives here such as adding a weak attribute to flash_init will
> not make us able to call the generic/original function. Therefore we
> consider adding an ifdef as better design than making the function
> weak, and it'll reduce the amount of redundant code in board code.
Why don't you think that you can't access the original function if it's
defined as a weak default? This should work just fine, see for example
ft_board_setup() in arch/powerpc/cpu/ppc4xx/fdt.c:
void __ft_board_setup(void *blob, bd_t *bd)
{
...
}
void ft_board_setup(void *blob, bd_t *bd) __attribute__((weak,
alias("__ft_board_setup")));
And then this weak default is overridden and still referenced in
board/amcc/canyonlands/canyonlands.c:
void ft_board_setup(void *blob, bd_t *bd)
{
__ft_board_setup(blob, bd);
...
So no need for this ifdef in the common CFI driver. Or am I missing something
here?
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: off...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
