From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Wed, 15 Apr 2009 10:22:36 +0200 Subject: [U-Boot] Question about M29W128G CFI QRY bug In-Reply-To: <49E51318.9050701@RuggedCom.com> References: <49DE84CE.4030103@RuggedCom.com> <200904141143.18280.sr@denx.de> <49E51318.9050701@RuggedCom.com> Message-ID: <200904151022.36195.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 Richard, On Wednesday 15 April 2009, Richard Retanubun wrote: > > Did you take a look at the CONFIG_SYS_FLASH_CFI_AMD_RESET define? I > > suggest > > > > you give it a try and let me know if this helps. > > This does help but for the wrong reason: > > #ifdef CONFIG_SYS_FLASH_CFI_AMD_RESET /* needed for STM_ID_29W320DB on > UC100 */ # undef FLASH_CMD_RESET > # define FLASH_CMD_RESET AMD_CMD_RESET /* use AMD-Reset instead */ > #endif > > This just makes the reset code send two AMD_CMD_RESET. And do you need an Intel reset command on your board? Is there an option for Intel command style FLASH chips? > > How is this problem solved in the Linux MTD driver (referring to your > > first email and the link you posted there)? > > The Linux way, it is addressed with this patch > ============================================== > http://lists.infradead.org/pipermail/linux-mtd/2008-August/022499.html > > Which does this: > > static int __xipram cfi_probe_chip(struct map_info *map, __u32 base, > unsigned long *chip_map, struct cfi_private *cfi) > @@ -116,11 +87,7 @@ static int __xipram cfi_probe_chip(struct map_info > *map, __u32 base, } > > xip_disable(); > > >> This is the code that is similar to our cfi-flash code << > > - cfi_send_gen_cmd(0xF0, 0, base, map, cfi, cfi->device_type, NULL); > - cfi_send_gen_cmd(0xFF, 0, base, map, cfi, cfi->device_type, NULL); > - cfi_send_gen_cmd(0x98, 0x55, base, map, cfi, cfi->device_type, NULL); > - > - if (!qry_present(map,base,cfi)) { > + if (!qry_mode_on(base, map, cfi)) { > xip_enable(base, map, cfi); > return 0; > } > > The new qry_mode_on function handles several "odd" flash cases (yeah, more > of these... *sob*) > > The quick and dirty way > ==================================================== > 1. Swap the order: (suggested in > http://lists.infradead.org/pipermail/linux-mtd/2008-July/022252.html ) > flash_write_cmd (info, 0, 0, FLASH_CMD_RESET); > flash_write_cmd (info, 0, 0, AMD_CMD_RESET); > > 2. Follow AMD_CMD_RESET with a FLASH_CMD_RESET: (Suggested by an e-mail > from Numonyx) flash_write_cmd (info, 0, 0, AMD_CMD_RESET); > flash_write_cmd (info, 0, 0, FLASH_CMD_RESET); > flash_write_cmd (info, 0, 0, AMD_CMD_RESET); > > I tested both methods and they both work on Numonyx M29W128GH (AMD cmdset) > and Numonyx PC28F320J3D75 (Intel cmdset) I don't like both versions. We should implement something which doesn't change the current behavior probably needed on some other boards. So how about something like this: === Cut here ==== diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 175d82a..c370a64 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -1683,15 +1683,25 @@ static void flash_read_cfi (flash_info_t *info, void *buf, p[i] = flash_read_uchar(info, start + i); } +void __flash_cmd_reset(flash_info_t *info) +{ + /* + * We do not yet know what kind of commandset to use, so we issue + * the reset command in both Intel and AMD variants, in the hope + * that AMD flash roms ignore the Intel command. + */ + flash_write_cmd(info, 0, 0, AMD_CMD_RESET); + flash_write_cmd(info, 0, 0, FLASH_CMD_RESET); +} +void flash_cmd_reset(flash_info_t *info) + __attribute__((weak,alias("__flash_cmd_reset"))); + static int __flash_detect_cfi (flash_info_t * info, struct cfi_qry *qry) { int cfi_offset; - /* We do not yet know what kind of commandset to use, so we issue - the reset command in both Intel and AMD variants, in the hope - that AMD flash roms ignore the Intel command. */ - flash_write_cmd (info, 0, 0, AMD_CMD_RESET); - flash_write_cmd (info, 0, 0, FLASH_CMD_RESET); + /* Issue FLASH reset command */ + flash_cmd_reset(info); for (cfi_offset=0; cfi_offset < sizeof(flash_offset_cfi) / sizeof(uint); === Cut here ==== By introducing a "weak" default reset function, you could overwrite it with your own board specific version. > For our local development, I'm probably going to adopt method #1 or method > #2. If either method #1 or #2 have a chance to be mainlined, I can submit a > patch for them with explanations. > > Obviously, the Linux method is probably the better solution (until another > odd part comes along), the porting of the linux method seems like it will > require a lot of time and testing, more than I am capable of committing > right now. Understood. 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 =====================================================================