[U-Boot] [PATCH 0/1] Fix hang trying to protect flash sectors

[mark tomlinson]

Yes we do have 2 flash chips. Here's the mapping: 

#define CONFIG_SYS_FLASH_BASE   0xf8000000  /* 2 chips*16M */ 
#define CONFIG_SYS_MONITOR_BASE  TEXT_BASE  /* start of monitor */ 

and in our config.mk file: 

TEXT_BASE = 0xfff40000 

This is the same flash chip as that at 0xf8000000, but remapped at reset by a CPLD to the high memory area too. 

The conditional code in cfi_flash.c: 
#if (CONFIG_SYS_MONITOR_BASE >= CONFIG_SYS_FLASH_BASE) && \ 
(!defined(CONFIG_MONITOR_IS_IN_RAM)) 
is therefore included since 0xfff40000 is greater than 0xf8000000, but flash_get_info(0xfff40000) returns NULL (as expected). 

I understand that not passing NULL to flash_protect() would be a better idea, and there's nothing wrong with doing both. I was going to fix it in cfi_flash.c, but noticed that many other areas of code (in different flash.c files) do the same thing. In our own build, I have just removed the code that tries to protect the monitor area, and will use an auto-protect area instead to do the same job. 

 - Mark 

>>> Stefan Roese <sr at denx.de> 5/18/2010 8:20 PM >>>
Hi Mark,

On Tuesday 18 May 2010 07:26:34 Mark Tomlinson wrote:
> Our hardware has part of the flash mapped in two address ranges.
> The CONFIG_SYS_MONITOR_BASE is in the upper 'boot' area, whereas
> the CONFIG_SYS_FLASH_BANKS_LIST has the full flash available at
> a lower address.

Just to be sure: You have 2 FLASH chips? Mapped at which addresses? And where
does CONFIG_SYS_MONITOR_BASE point to?

> This all works fine until the code in cfi_flash.c:flash_init(), which
> uses flash_get_info() to find the flash_info_t associated with the
> monitor and environment. These are not in the probed flash range, so
> flash_get_info() returns NULL.

You mean that "flash_get_info(CONFIG_SYS_MONITOR_BASE)" returns NULL? Please
explain again, why is this the case?

> This is not checked and is passed
> directly to flash_protect(). Since flash_protect() was not checking
> for this NULL pointer either, random memory would be clobbered
> causing the device to lock up.
>
> This patch changes flash_protect() to check for the NULL, and the
> error goes unreported.

I would prefer to fix the real problem, that flash_get_info() returns NULL,
instead.

Cheers,
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 at denx.de

NOTICE: This message contains privileged and confidential
information intended only for the use of the addressee
named above. If you are not the intended recipient of
this message you are hereby notified that you must not
disseminate, copy or take any action in reliance on it.
If you have received this message in error please
notify Allied Telesis Labs Ltd immediately.
Any views expressed in this message are those of the
individual sender, except where the sender has the
authority to issue and specifically states them to
be the views of Allied Telesis Labs.