On Tuesday 06 November 2007, Michael Schwingen wrote:
On Mon, Nov 05, 2007 at 12:21:50PM +0100, Stefan Roese wrote:
I don't like the indentation problem we get from this #ifdef here. Perhaps we should add a __weak__ function flash_detect_legacy() in this file, that can be overridden by board specific functions. Like this:
ulong __flash_detect_legacy(ulong base, int banknum) { return 0; } ulong flash_detect_legacy(ulong base, int banknum) __attribute__((weak, alias("__flash_detect_legacy")));
This way we get rid of the #ifdef too.
Hm - I need some common code at that point, which would otherwise be duplicated in every board code. I have now moved the code to a function flash_detect_legacy.
OK.
for (j = 0; j < erase_region_count; j++) {
if (sect_cnt >= CFG_MAX_FLASH_SECT)break;Please add an error output here too.
Done.
Okey, next version. The board-specific code may either fill out the complete flash_info struct as in the previous patch, or (preferred), only set info->portwidth, info->chipwidth and info->interface, in which case the code will probe the flash by jedec IDs and look up a table in jedec_flash.c. The table is a near copy of the table in the Linux jedec_flash.c, with the removal of some unused fields.
Wouldn't it be better not to remove those unused fields? This way porting of new devices from Linux to U-Boot would be easier.
There is one problem with the AMD_ADDR_* definitions: I believe they are only correct for 16-bit flash ROMS (in 8/16 bit mode), but are wrong for 8-bit flashs. I think the CFI "interface" parameter should be the correct way to distinguish these cases. What is left is the number of bits in the unlock addresses - I have used 0x2AAA/0x5555, since all flash roms I know treat the upper bits as "don't care" (and specify that behaviour in the datasheet), so if the flash datasheet specifies 0xAAA/0x555, using the longer constants should do no harm. However, there are probably lots of flash roms I do *not* know, so I would appreciate feedback on this. If different addresses are really necessary, we would need to add them to the flash_info struct.
Correct. And this is the case in the current Linux implementation. So we should not remove it, even if we don't use it for now.
This currently works on my IXP425 board (big endian) with one SST39VF020 and one Intel TE28F640J3. I am not sure if the jedec flash code is correct in case of 16-bit JEDEC flashs or multiple 8-bit flash roms on a wider data bus.
Thanks. Very nice work.
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: office@denx.de =====================================================================