Hi Michael,
On Saturday 03 November 2007, Michael Schwingen wrote:
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.).
Yes, I think this is ok for now.
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.
Good catch. I remember seeing a patch dealing with this problem before.
Please find some first comments below.
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;
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.
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);@@ -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;
Please add an error output here too.
Looks good otherwise. Thanks.
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 =====================================================================