[U-Boot] Question about M29W128G CFI QRY bug
Stefan Roese
sr at denx.de
Wed Apr 15 10:22:36 CEST 2009
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
=====================================================================
More information about the U-Boot
mailing list