From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Mon, 5 Nov 2007 12:21:50 +0100 Subject: [U-Boot-Users] Mixing CFI and non-CFI flashs? In-Reply-To: <20071103150055.GA17224@discworld.dascon.de> References: <472B8E46.7000808@discworld.dascon.de> <200711030813.40329.sr@denx.de> <20071103150055.GA17224@discworld.dascon.de> Message-ID: <200711051221.50299.sr@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > 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); > 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; 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 at denx.de =====================================================================