| Message ID | 1322817437-4955-2-git-send-email-Shengzhou.Liu@freescale.com |
|---|---|
| State | Superseded |
| Delegated to: | Scott Wood |
| Headers | show |
On 12/02/2011 03:17 AM, Shengzhou Liu wrote: > There was a bug logically in the order of nand_flash_detect_onfi > and checking nand_flash_ids. We should get NAND devices related > informations first by ONFI method instead of querying nand_flash_ids table, > if ONFI fails, then query nand_flash_ids table. ONFI issues should be taken care of with the patchset to sync NAND with Linux that was recently posted. It has some minor issues, which hopefully will be addressed by the next merge window. I disagree that we should check ONFI first -- this is not what Linux does. U-Boot's problem is that it aborts before ever checking ONFI, if the device is not found in the table (and the Linux code suggests that ONFI devices won't be found in the table). -Scott
On 12/03/2011 03:11 AM, Liu Shengzhou-B36685 wrote: >> -----Original Message----- >> From: Wood Scott-B07421 >> Sent: 2011年12月3日 2:32 >> To: Liu Shengzhou-B36685 >> Cc: u-boot@lists.denx.de; Gala Kumar-B11780 >> Subject: Re: [PATCH 2/4] mtd/nand: Fixup for support ONFI detect >> >> On 12/02/2011 03:17 AM, Shengzhou Liu wrote: >>> There was a bug logically in the order of nand_flash_detect_onfi >>> and checking nand_flash_ids. We should get NAND devices related >>> informations first by ONFI method instead of querying nand_flash_ids >> table, >>> if ONFI fails, then query nand_flash_ids table. >> >> ONFI issues should be taken care of with the patchset to sync NAND with >> Linux that was recently posted. It has some minor issues, which >> hopefully will be addressed by the next merge window. >> >> I disagree that we should check ONFI first -- this is not what Linux >> does. U-Boot's problem is that it aborts before ever checking ONFI, if >> the device is not found in the table (and the Linux code suggests that >> ONFI devices won't be found in the table). >> >> -Scott > > Currently in Linux, ONFI is detected first, if ONFI fails, then detect nand_flash_ids table. How do you get that impression from this code:? for (; type->name != NULL; type++) if (*dev_id == type->id) break; chip->onfi_version = 0; if (!type->name || !type->pagesize) { /* Check is chip is ONFI compliant */ ret = nand_flash_detect_onfi(mtd, chip, &busw); if (ret) goto ident_done; } nand_flash_detect_onfi() is called only if there is no match found in the ID table. > But in u-boot, it first detects nand_flash_ids table, if not found in the table, it aborts, so ONFI never works. Right, the problem is that it aborts, not that it checks the table first. > I think we should still check ONFI first to accord to what Linux > does, because there many new NAND chips which not listed in the > nand_flash_ids table before known, if check ONFI first, it can > automatically find related info without manual intervention. The assumption in the Linux code appears to be that ONFI chips will never be in the ID table (or possibly that if they are, we don't need the ONFI data). -Scott
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 6aac6a2..8d03f54 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -2585,36 +2585,38 @@ static const struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd, return ERR_PTR(-ENODEV); } - if (!type) - type = nand_flash_ids; + ret = nand_flash_detect_onfi(mtd, chip, &busw); + if (!ret) { + if (!type) + type = nand_flash_ids; - for (; type->name != NULL; type++) - if (*dev_id == type->id) - break; + for (; type->name != NULL; type++) + if (*dev_id == type->id) + break; - if (!type->name) { - /* supress warning if there is no nand */ - if (*maf_id != 0x00 && *maf_id != 0xff && - *dev_id != 0x00 && *dev_id != 0xff) - printk(KERN_INFO "%s: unknown NAND device: " - "Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n", - __func__, *maf_id, *dev_id); - return ERR_PTR(-ENODEV); - } + if (!type->name) { + /* supress warning if there is no nand */ + if (*maf_id != 0x00 && *maf_id != 0xff && + *dev_id != 0x00 && *dev_id != 0xff) { + printk(KERN_INFO "%s: unknown NAND device: " + "Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n", + __func__, *maf_id, *dev_id); + } + return ERR_PTR(-ENODEV); + } - if (!mtd->name) - mtd->name = type->name; + if (!mtd->name) + mtd->name = type->name; - chip->chipsize = (uint64_t)type->chipsize << 20; - chip->onfi_version = 0; + chip->chipsize = (uint64_t)type->chipsize << 20; + chip->onfi_version = 0; - ret = nand_flash_detect_onfi(mtd, chip, &busw); - if (!ret) nand_flash_detect_non_onfi(mtd, chip, type, &busw); - /* Get chip options, preserve non chip based options */ - chip->options &= ~NAND_CHIPOPTIONS_MSK; - chip->options |= type->options & NAND_CHIPOPTIONS_MSK; + /* Get chip options, preserve non chip based options */ + chip->options &= ~NAND_CHIPOPTIONS_MSK; + chip->options |= type->options & NAND_CHIPOPTIONS_MSK; + } /* * Set chip as a default. Board drivers can override it, if necessary
There was a bug logically in the order of nand_flash_detect_onfi and checking nand_flash_ids. We should get NAND devices related informations first by ONFI method instead of querying nand_flash_ids table, if ONFI fails, then query nand_flash_ids table. Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com> --- drivers/mtd/nand/nand_base.c | 48 +++++++++++++++++++++-------------------- 1 files changed, 25 insertions(+), 23 deletions(-)