Mark, On Tuesday 18 May 2010 22:10:51 mark tomlinson wrote: > Yes we do have 2 flash chips. Here's the mapping: > > #define CONFIG_SYS_FLASH_BASE 0xf8000000 /* 2 chips*16M */
Hmmm. 2 * 16MByte, thats 32MByte == 0x2000000. So you should have one chip at base address 0xff000000 and one at 0xfe000000. Why 0xf8000000? > #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. This seems wrong. See my comments above. > 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 don't see why flash_get_info(0xfff40000) should return NULL. It should return the pointer to the 16MB FLASH chip starting at 0xff000000. > I understand that not passing NULL to flash_protect() would be a better > idea, and there's nothing wrong with doing both. Agreed in general. But we have to keep the code compact. And unnecessary checks do increase the code size (at least a small bit). > 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. "auto-protect area"? Please explain what you mean with this? Perhaps this is an interesting "feature" for mainline as well. 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: off...@denx.de _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
