On Sat, Nov 03, 2007 at 08:13:40AM +0100, Stefan Roese wrote:

I remember a discussion we had a while ago, about supporting non-CFI chips in a common driver, but I can't find the mails right now. IIRC, the idea was to add something close to the Linux JEDEC probe (drivers/mtd/chips/jedec_probe.c) for this.

Good idea. I currently hard-coded the flash parameters for the boot flash, since I know there is exactly one type on the boards, but having jedec-probing would be nicer.

I think the CFI driver is a good start as a framework - all the infrastructure for handling mutliple banks, different chip/bus mappings etc. is there and could be used by jedec-flash code. For now, it is probably easiest to add the JEDEC code to cfi_flash.c, to get access to the static functions defined there, and then later split it out if necessary.

I suggest that you post your current version for review. This sounds like a start in the correct direction.

Fine. Here is what I have got now - it works for my board, but would probably need some cleanup/renaming of symbols. The basic idea is that the board-specific code provides a function "flash_detect_legacy" that is called for every flash bank, which can provide hardcoded (or in the future jedec-probed) flash information. If the function returns 0, the normal CFI code is called.

Since that code is board-specific anyway, I think the board-specific code can supply the information about chipwidth, buswidth etc. - there is no real benefit in autoprobing these, and there is less probability for problems (like 8-bit accesses to a 16-bit bus causing problems, etc.).

I have included one non-conditional patch I made: @@ -1282,6 +1310,8 @@ ulong flash_get_size (ulong base, int banknum) which causes the CFI code to stop in case CFG_MAX_FLASH_SECT is defined too small - without that patch, u-boot overwrites other data and crashes silently during startup.

cu Michael

Signed-off-by: Michael Schwingen michael@schwingen.org

diff --git a/include/flash.h b/include/flash.h index b0bf733..f0ef6f7 100644 --- a/include/flash.h +++ b/include/flash.h @@ -101,6 +101,11 @@ extern void flash_read_user_serial(flash_info_t * info, void * buffer, int offse extern void flash_read_factory_serial(flash_info_t * info, void * buffer, int offset, int len); #endif /* CFG_FLASH_PROTECTION */

+#ifdef CFG_FLASH_CFI_LEGACY +extern ulong flash_detect_legacy(ulong base, int banknum); +#define CFI_CMDSET_AMD_LEGACY 0xFFF0 +#endif + /*----------------------------------------------------------------------- * return codes from flash_write(): */

diff --git a/drivers/cfi_flash.c b/drivers/cfi_flash.c index 5579a1e..20ccc3a 100644 --- a/drivers/cfi_flash.c +++ b/drivers/cfi_flash.c @@ -345,7 +345,13 @@ unsigned long flash_init (void) /* Init: no FLASHes known */ for (i = 0; i < CFG_MAX_FLASH_BANKS; ++i) { flash_info[i].flash_id = FLASH_UNKNOWN; - size += flash_info[i].size = flash_get_size (bank_base[i], i); + +#ifdef CFG_FLASH_CFI_LEGACY + flash_info[i].size = flash_detect_legacy (bank_base[i], i); + if (flash_info[i].size == 0) +#endif + flash_info[i].size = flash_get_size (bank_base[i], i); + size += flash_info[i].size; if (flash_info[i].flash_id == FLASH_UNKNOWN) { #ifndef CFG_FLASH_QUIET_TEST printf ("## Unknown FLASH on Bank %d - Size = 0x%08lx = %ld MB\n", @@ -488,6 +494,16 @@ int flash_erase (flash_info_t * info, int s_first, int s_last) flash_unlock_seq (info, sect); flash_write_cmd (info, sect, 0, AMD_CMD_ERASE_SECTOR); break; +#ifdef CFG_FLASH_CFI_LEGACY + case CFI_CMDSET_AMD_LEGACY: + flash_write_cmd (info, 0, 0x5555, AMD_CMD_UNLOCK_START); + flash_write_cmd (info, 0, 0x2AAA, AMD_CMD_UNLOCK_ACK); + flash_write_cmd (info, 0, 0x5555, AMD_CMD_ERASE_START); + flash_write_cmd (info, 0, 0x5555, AMD_CMD_UNLOCK_START); + flash_write_cmd (info, 0, 0x2AAA, AMD_CMD_UNLOCK_ACK); + flash_write_cmd (info, sect, 0, AMD_CMD_ERASE_SECTOR); + break; +#endif default: debug ("Unkown flash vendor %d\n", info->vendor); @@ -518,8 +534,12 @@ void flash_print_info (flash_info_t * info)

printf ("CFI conformant FLASH (%d x %d)", (info->portwidth << 3), (info->chipwidth << 3)); - printf (" Size: %ld MB in %d Sectors\n", - info->size >> 20, info->sector_count); + if (info->size < 1024*1024) + printf (" Size: %ld kB in %d Sectors\n", + info->size >> 10, info->sector_count); + else + printf (" Size: %ld MB in %d Sectors\n", + info->size >> 20, info->sector_count); printf (" "); switch (info->vendor) { case CFI_CMDSET_INTEL_STANDARD: @@ -534,6 +554,11 @@ void flash_print_info (flash_info_t * info) case CFI_CMDSET_AMD_EXTENDED: printf ("AMD Extended"); break; +#ifdef CFG_FLASH_CFI_LEGACY + case CFI_CMDSET_AMD_LEGACY: + printf ("AMD (non-CFI)"); + break; +#endif default: printf ("Unknown (%d)", info->vendor); break; @@ -777,6 +802,9 @@ static int flash_is_busy (flash_info_t * info, flash_sect_t sect) break; case CFI_CMDSET_AMD_STANDARD: case CFI_CMDSET_AMD_EXTENDED: +#ifdef CFG_FLASH_CFI_LEGACY + case CFI_CMDSET_AMD_LEGACY: +#endif retval = flash_toggle (info, sect, 0, AMD_STATUS_TOGGLE); break; default: @@ -1282,6 +1310,8 @@ ulong flash_get_size (ulong base, int banknum) debug ("erase_region_count = %d erase_region_size = %d\n", erase_region_count, erase_region_size); for (j = 0; j < erase_region_count; j++) { + if (sect_cnt >= CFG_MAX_FLASH_SECT) + break; info->start[sect_cnt] = sector; sector += (erase_region_size * size_ratio);

@@ -1387,6 +1417,12 @@ static int flash_write_cfiword (flash_info_t * info, ulong dest, flash_unlock_seq (info, 0); flash_write_cmd (info, 0, AMD_ADDR_START, AMD_CMD_WRITE); break; +#ifdef CFG_FLASH_CFI_LEGACY + case CFI_CMDSET_AMD_LEGACY: + flash_write_cmd (info, 0, 0x5555, AMD_CMD_UNLOCK_START); + flash_write_cmd (info, 0, 0x2AAA, AMD_CMD_UNLOCK_ACK); + flash_write_cmd (info, 0, 0x5555, AMD_CMD_WRITE); +#endif }

switch (info->portwidth) {

and then in the board-init code:

extern flash_info_t flash_info[]; #define AMD_CMD_RESET 0xF0

ulong flash_detect_legacy(ulong base, int banknum) { if (banknum == 0) { flash_info_t *info = &flash_info[banknum]; int i; unsigned long sector;

info->portwidth = 1; info->chipwidth = 1; info->ext_addr = 0; info->cfi_version = 0; info->legacy_unlock = 0; info->start[0] = base; info->cmd_reset = AMD_CMD_RESET; info->vendor = CFI_CMDSET_AMD_LEGACY; info->flash_id = FLASH_MAN_CFI;

info->sector_count = 64;

for (i = 0, sector = base; i < info->sector_count; i++) { info->protect[i] = 0; info->start[i] = sector; sector += 4096; }

info->buffer_size = 1; info->erase_blk_tout = 10000; info->buffer_write_tout = 1000; info->write_tout = 100; return 256*1024; } else return 0; }

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.

...

	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.

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.

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.

Signed-off-by: Michael Schwingen michael@schwingen.org

diff --git a/drivers/cfi_flash.c b/drivers/cfi_flash.c index 5579a1e..399deab 100644 --- a/drivers/cfi_flash.c +++ b/drivers/cfi_flash.c @@ -98,9 +98,9 @@ #define AMD_STATUS_TOGGLE 0x40 #define AMD_STATUS_ERROR 0x20

-#define AMD_ADDR_ERASE_START ((info->portwidth == FLASH_CFI_8BIT) ? 0xAAA : 0x555) -#define AMD_ADDR_START ((info->portwidth == FLASH_CFI_8BIT) ? 0xAAA : 0x555) -#define AMD_ADDR_ACK ((info->portwidth == FLASH_CFI_8BIT) ? 0x555 : 0x2AA) +#define AMD_ADDR_ERASE_START ((info->portwidth == FLASH_CFI_8BIT && info->interface == FLASH_CFI_X8X16) ? 0x2AAA : 0x5555) +#define AMD_ADDR_START ((info->portwidth == FLASH_CFI_8BIT && info->interface == FLASH_CFI_X8X16) ? 0x2AAA : 0x5555) +#define AMD_ADDR_ACK ((info->portwidth == FLASH_CFI_8BIT && info->interface == FLASH_CFI_X8X16) ? 0x5555 : 0x2AAA)

#define FLASH_OFFSET_MANUFACTURER_ID 0x00 #define FLASH_OFFSET_DEVICE_ID 0x01 @@ -331,6 +331,43 @@ ulong flash_read_long (flash_info_t * info, flash_sect_t sect, uint offset) }

+#ifdef CFG_FLASH_CFI_LEGACY +ulong flash_detect_legacy(ulong base, int banknum) +{ + flash_info_t *info = &flash_info[banknum]; + if (board_flash_get_legacy(base, banknum, info)) { + /* board code may have filled info completely. If not, we + use JEDEC ID probing. */ + if (!info->vendor) { + int modes[] = { CFI_CMDSET_AMD_STANDARD, CFI_CMDSET_INTEL_STANDARD }; + int i; + + for(i=0; i<sizeof(modes)/sizeof(modes[0]); i++) { + info->vendor = modes[i]; + info->start[0] = base; + flash_read_jedec_ids(info); + debug("JEDEC PROBE: ID %x %x %x\n", info->manufacturer_id, info->device_id, info->device_id2); + if (jedec_flash_match(info, base)) + break; + } + } + switch(info->vendor) { + case CFI_CMDSET_AMD_LEGACY: + info->cmd_reset = AMD_CMD_RESET; + break; + } + info->flash_id = FLASH_MAN_CFI; + } + return info->size; +} +#else +ulong inline flash_detect_legacy(ulong base, int banknum) +{ + return 0; +} +#endif + + /*----------------------------------------------------------------------- */ unsigned long flash_init (void) @@ -345,7 +382,11 @@ unsigned long flash_init (void) /* Init: no FLASHes known */ for (i = 0; i < CFG_MAX_FLASH_BANKS; ++i) { flash_info[i].flash_id = FLASH_UNKNOWN; - size += flash_info[i].size = flash_get_size (bank_base[i], i); + + flash_detect_legacy (bank_base[i], i); + if (flash_info[i].size == 0) + flash_info[i].size = flash_get_size (bank_base[i], i); + size += flash_info[i].size; if (flash_info[i].flash_id == FLASH_UNKNOWN) { #ifndef CFG_FLASH_QUIET_TEST printf ("## Unknown FLASH on Bank %d - Size = 0x%08lx = %ld MB\n", @@ -488,6 +529,16 @@ int flash_erase (flash_info_t * info, int s_first, int s_last) flash_unlock_seq (info, sect); flash_write_cmd (info, sect, 0, AMD_CMD_ERASE_SECTOR); break; +#ifdef CFG_FLASH_CFI_LEGACY + case CFI_CMDSET_AMD_LEGACY: + flash_write_cmd (info, 0, 0x5555, AMD_CMD_UNLOCK_START); + flash_write_cmd (info, 0, 0x2AAA, AMD_CMD_UNLOCK_ACK); + flash_write_cmd (info, 0, 0x5555, AMD_CMD_ERASE_START); + flash_write_cmd (info, 0, 0x5555, AMD_CMD_UNLOCK_START); + flash_write_cmd (info, 0, 0x2AAA, AMD_CMD_UNLOCK_ACK); + flash_write_cmd (info, sect, 0, AMD_CMD_ERASE_SECTOR); + break; +#endif default: debug ("Unkown flash vendor %d\n", info->vendor); @@ -518,8 +569,12 @@ void flash_print_info (flash_info_t * info)

printf ("CFI conformant FLASH (%d x %d)", (info->portwidth << 3), (info->chipwidth << 3)); - printf (" Size: %ld MB in %d Sectors\n", - info->size >> 20, info->sector_count); + if (info->size < 1024*1024) + printf (" Size: %ld kB in %d Sectors\n", + info->size >> 10, info->sector_count); + else + printf (" Size: %ld MB in %d Sectors\n", + info->size >> 20, info->sector_count); printf (" "); switch (info->vendor) { case CFI_CMDSET_INTEL_STANDARD: @@ -534,6 +589,11 @@ void flash_print_info (flash_info_t * info) case CFI_CMDSET_AMD_EXTENDED: printf ("AMD Extended"); break; +#ifdef CFG_FLASH_CFI_LEGACY + case CFI_CMDSET_AMD_LEGACY: + printf ("AMD (non-CFI)"); + break; +#endif default: printf ("Unknown (%d)", info->vendor); break; @@ -777,6 +837,9 @@ static int flash_is_busy (flash_info_t * info, flash_sect_t sect) break; case CFI_CMDSET_AMD_STANDARD: case CFI_CMDSET_AMD_EXTENDED: +#ifdef CFG_FLASH_CFI_LEGACY + case CFI_CMDSET_AMD_LEGACY: +#endif retval = flash_toggle (info, sect, 0, AMD_STATUS_TOGGLE); break; default: @@ -1282,6 +1345,10 @@ ulong flash_get_size (ulong base, int banknum) debug ("erase_region_count = %d erase_region_size = %d\n", erase_region_count, erase_region_size); for (j = 0; j < erase_region_count; j++) { + if (sect_cnt >= CFG_MAX_FLASH_SECT) { + printf("ERROR: too many flash sectors\n"); + break; + } info->start[sect_cnt] = sector; sector += (erase_region_size * size_ratio);

@@ -1387,6 +1454,12 @@ static int flash_write_cfiword (flash_info_t * info, ulong dest, flash_unlock_seq (info, 0); flash_write_cmd (info, 0, AMD_ADDR_START, AMD_CMD_WRITE); break; +#ifdef CFG_FLASH_CFI_LEGACY + case CFI_CMDSET_AMD_LEGACY: + flash_write_cmd (info, 0, 0x5555, AMD_CMD_UNLOCK_START); + flash_write_cmd (info, 0, 0x2AAA, AMD_CMD_UNLOCK_ACK); + flash_write_cmd (info, 0, 0x5555, AMD_CMD_WRITE); +#endif }

switch (info->portwidth) { diff --git a/include/flash.h b/include/flash.h index b0bf733..c4d8ded 100644 --- a/include/flash.h +++ b/include/flash.h @@ -101,6 +101,12 @@ extern void flash_read_user_serial(flash_info_t * info, void * buffer, int offse extern void flash_read_factory_serial(flash_info_t * info, void * buffer, int offset, int len); #endif /* CFG_FLASH_PROTECTION */

+#ifdef CFG_FLASH_CFI_LEGACY +extern ulong board_flash_get_legacy(ulong base, int banknum, flash_info_t *info); +extern int jedec_flash_match(flash_info_t *info, ulong base); +#define CFI_CMDSET_AMD_LEGACY 0xFFF0 +#endif + /*----------------------------------------------------------------------- * return codes from flash_write(): */

new file jedec_flash.c:

/* * (C) Copyright 2007 * Michael Schwingen, michael@schwingen.org * * based in great part on jedec_probe.c from linux kernel: * (C) 2000 Red Hat. GPL'd. * Occasionally maintained by Thayne Harbaugh tharbaugh at lnxi dot com * * See file CREDITS for list of people who contributed to this * project. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as * published by the Free Software Foundation; either version 2 of * the License, or (at your option) any later version. * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, * MA 02111-1307 USA * */

/* The DEBUG define must be before common to enable debugging */ #define DEBUG

#include <common.h> #include <asm/processor.h> #include <asm/io.h> #include <asm/byteorder.h> #include <environment.h>

#if defined CFG_FLASH_CFI_DRIVER && defined CFG_FLASH_CFI_LEGACY

#define P_ID_AMD_STD CFI_CMDSET_AMD_LEGACY

/* Manufacturers */ #define MANUFACTURER_SST 0x00BF

/* SST */ #define SST39LF800 0x2781 #define SST39LF160 0x2782 #define SST39VF1601 0x234b #define SST39LF512 0x00D4 #define SST39LF010 0x00D5 #define SST39LF020 0x00D6 #define SST39LF040 0x00D7 #define SST39SF010A 0x00B5 #define SST39SF020A 0x00B6

struct amd_flash_info { const __u16 mfr_id; const __u16 dev_id; const char *name; const int NumEraseRegions; const int CmdSet; const ulong regions[6]; };

#define ERASEINFO(size,blocks) (size<<8)|(blocks-1)

/* * Please keep this list ordered by manufacturer! * Fortunately, the list isn't searched often and so a * slow, linear search isn't so bad. */ static const struct amd_flash_info jedec_table[] = { { .mfr_id = MANUFACTURER_SST, .dev_id = SST39LF010, .name = "SST 39LF010", .CmdSet = P_ID_AMD_STD, .NumEraseRegions= 1, .regions = { ERASEINFO(0x01000,32), } }, { .mfr_id = MANUFACTURER_SST, .dev_id = SST39LF020, .name = "SST 39LF020", .CmdSet = P_ID_AMD_STD, .NumEraseRegions= 1, .regions = { ERASEINFO(0x01000,64), } }, { .mfr_id = MANUFACTURER_SST, .dev_id = SST39LF040, .name = "SST 39LF040", .CmdSet = P_ID_AMD_STD, .NumEraseRegions= 1, .regions = { ERASEINFO(0x01000,128), } } };

#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

static inline void fill_info(flash_info_t *info, const struct amd_flash_info *jedec_entry, ulong base) { int i,j; int sect_cnt; int size_ratio; int total_size;

size_ratio = info->portwidth / info->chipwidth;

debug("Found JEDEC Flash: %s\n", jedec_entry->name); info->vendor = jedec_entry->CmdSet; /* Todo: do we need device-specific timeouts? */ info->erase_blk_tout = 30000; info->buffer_write_tout = 1000; info->write_tout = 100;

sect_cnt = 0; total_size = 0; for (i = 0; i < jedec_entry->NumEraseRegions; i++) { ulong erase_region_size = jedec_entry->regions[i] >> 8; ulong erase_region_count = (jedec_entry->regions[i] & 0xff) + 1;

total_size += erase_region_size * erase_region_count; debug ("erase_region_count = %d erase_region_size = %d\n", erase_region_count, erase_region_size); for (j = 0; j < erase_region_count; j++) { if (sect_cnt >= CFG_MAX_FLASH_SECT) { printf("ERROR: too many flash sectors\n"); break; } info->start[sect_cnt] = base; base += (erase_region_size * size_ratio); sect_cnt++; } } info->sector_count = sect_cnt; info->size = total_size * size_ratio; }

/*----------------------------------------------------------------------- * match jedec ids against table. If a match is found, fill flash_info entry */ int jedec_flash_match(flash_info_t *info, ulong base) { int ret = 0; int i; ulong mask = 0xFFFF; if (info->chipwidth == 1) mask = 0xFF;

for (i = 0; i < ARRAY_SIZE(jedec_table); i++) { if ( (jedec_table[i].mfr_id & mask) == (info->manufacturer_id & mask) && (jedec_table[i].dev_id & mask) == (info->device_id & mask)) { fill_info(info, &jedec_table[i], base); ret = 1; break; } } return ret; }

#endif /* defined CFG_FLASH_CFI_DRIVER && defined CFG_FLASH_CFI_LEGACY */

And finally, the board-specific code:

ulong board_flash_get_legacy(ulong base, int banknum, flash_info_t *info) { if (banknum == 0) { info->portwidth = 1; info->chipwidth = 1; info->interface = FLASH_CFI_X8; return 1; } else return 0; }

cu Michael