[U-Boot] Reg. CFI flash_init and hardware write protected devices

[Frank Svendsbøe]

Hi Stefan,

On Tue, May 31, 2011 at 4:37 PM, Stefan Roese <sr at denx.de> wrote:
> Hi Frank,
>
> On Tuesday 31 May 2011 15:55:56 Frank Svendsbøe wrote:
>> > 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?
>>
>> Simply because disabling write-protection is not impossible after
>> installation. Our device will be located 3000m below sea level.
>
> I see.
>

Hmm.. then you read my mind :) I meant to say "is not possible" and not "is not
impossible" :)

>> As I explained Mike Frysinger, the write-protection settings is not
>> controlled by the PPC device running U-Boot. We can enable
>> write-protection in the lab (by setting a jumper), but not write software.
>>

.. and here I meant to say "explained to", and "but not via software".


>> Two ways/levels: 1) A hardware jumper on the factory default flash. 2)
>> On the non-factory default flash, write protection is enabled/disabled
>> by an FPGA and implicitly and AVR. To make it short, we cannot
>> change protection scheme from U-Boot (but we can via an SPI driver I
>> wrote for Linux).
>

.. and yet another self-correction: s/and implicitly and/and implicitly an/
.. guess I had too much coffee yesterday :)

> Theoretically also possible with U-Boot. But I understand that you don't want
> to do this.
>

True. We could write a driver for U-Boot that could access the AVR and
command the FPGA to
disable write protection. But in the case for the "default factory
flash" where the flash
write protection is enabled by a demounted jumper, and cannot be
modified after installation,
CFI probing wouldn't work anyway. So I think the use for such a driver
would be limited.

Note that for normal operation, our system is running from a
non-hardware protected flash. But
even in this mode, we must command the AVR to enable write access
before we're permitted
to write to it. This is one of the features supported by the SPI
driver running in GNU/Linux.

In addition to this, we have the usual CFI/MTD protection scheme where
the filesystems
are mounted as read-only, etc.

> <snip>
>
>> > 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?
>>
>> Oh. I didn't knew I could access the function that was overridden by the
>> weak attribute. I guess that's the alias is for right?
>
> Yep.
>
>> If both can be
>> called, I'm happy to remove the ifdef.
>>
>> I'll test that tomorrow and provide a patch if it works.
>
> Good luck...
>

Thanks, this worked for me. Is it ok for you too?

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 6039e1f..99360af 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -176,6 +176,7 @@ u64 flash_read64(void *addr)__attribute__((weak,
alias("__flash_read64")));
 #define flash_read64	__flash_read64
 #endif

+
 /*-----------------------------------------------------------------------
  */
 #if defined(CONFIG_ENV_IS_IN_FLASH) ||
defined(CONFIG_ENV_ADDR_REDUND) || (CONFIG_SYS_MONITOR_BASE >=
CONFIG_SYS_FLASH_BASE)
@@ -2151,7 +2152,8 @@ void flash_protect_default(void)
 #endif
 }

-unsigned long flash_init (void)
+unsigned long flash_init(void) __attribute__((weak, alias("__flash_init")));
+unsigned long __flash_init (void)
 {
 	unsigned long size = 0;
 	int i;