| Message ID | 1368607232-2210-7-git-send-email-b32955@freescale.com |
|---|---|
| State | New, archived |
| Headers | show |
On 5/15/2013 2:10 PM, Huang Shijie wrote: > Since the ONFI 2.1, the onfi spec adds the Extended Parameter Page > to store the ECC info. > > The onfi spec tells us that if the nand chip's recommended ECC codeword > size is not 512 bytes, then the @ecc_bits is 0xff. The host _SHOULD_ then Section 3.4.2 also says, "If a value of 0 is reported then this ECC Information Block is invalid and should not be used". Is this taken care anywhere? > read the Extended ECC information that is part of the extended parameter > page to retrieve the ECC requirements for this device. > > This patch implement the reading of the Extended Parameter Page, and parses > the sections for ECC type, and get the ECC info from the ECC section. > > Tested this patch with Micron MT29F64G08CBABAWP. > > Acked-by: Pekon Gupta <pekon@ti.com> > Signed-off-by: Huang Shijie <b32955@freescale.com> > --- > drivers/mtd/nand/nand_base.c | 77 ++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 77 insertions(+), 0 deletions(-) > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > index 15630ef..e98ea69 100644 > --- a/drivers/mtd/nand/nand_base.c > +++ b/drivers/mtd/nand/nand_base.c > @@ -2835,6 +2835,68 @@ static u16 onfi_crc16(u16 crc, u8 const *p, size_t len) > return crc; > } > > +/* Parse the Extended Parameter Page. */ > +static int nand_flash_detect_ext_param_page(struct mtd_info *mtd, > + struct nand_chip *chip, struct nand_onfi_params *p) > +{ > + struct onfi_ext_param_page *ep; > + struct onfi_ext_section *s; > + struct onfi_ext_ecc_info *ecc; > + uint8_t *cursor; > + int len; > + int ret; > + int i; > + > + len = le16_to_cpu(p->ext_param_page_length) * 16; > + ep = kmalloc(len, GFP_KERNEL); > + if (!ep) { > + ret = -ENOMEM; > + goto ext_out; Just return -ENOMEM. Why do you free the memory when it is not allocated? (Though it doesn't cause any harm) > + } > + > + /* Send our own NAND_CMD_PARAM. */ > + chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1); > + > + /* Use the Change Read Column command to skip the ONFI param pages. */ > + chip->cmdfunc(mtd, NAND_CMD_RNDOUT, > + sizeof(*p) * p->num_of_param_pages , -1); > + > + /* Read out the Extended Parameter Page. */ > + chip->read_buf(mtd, (uint8_t *)ep, len); > + if ((onfi_crc16(ONFI_CRC_BASE, ((uint8_t *)ep) + 2, len - 2) > + != le16_to_cpu(ep->crc)) || strncmp(ep->sig, "EPPS", 4)) { > + pr_debug("fail in the CRC.\n"); > + ret = -EINVAL; > + goto ext_out; > + } > + > + /* find the ECC section. */ > + cursor = (uint8_t *)(ep + 1); > + for (i = 0; i < ONFI_EXT_SECTION_MAX; i++) { > + s = ep->sections + i; > + if (s->type == ONFI_SECTION_TYPE_2) > + break; > + cursor += s->length * 16; > + } > + if (i == ONFI_EXT_SECTION_MAX) { > + pr_debug("We can not find the ECC section.\n"); > + ret = -EINVAL; > + goto ext_out; > + } > + > + /* get the info we want. */ > + ecc = (struct onfi_ext_ecc_info *)cursor; > + chip->ecc_strength = ecc->ecc_bits; > + chip->ecc_step = 1 << ecc->codeword_size; > + > + pr_info("ONFI extended param page detected.\n"); > + return 0; > + > +ext_out: > + kfree(ep); > + return ret; > +} > + ~Vikram
于 2013年05月16日 00:57, Vikram Narayanan 写道: > On 5/15/2013 2:10 PM, Huang Shijie wrote: >> Since the ONFI 2.1, the onfi spec adds the Extended Parameter Page >> to store the ECC info. >> >> The onfi spec tells us that if the nand chip's recommended ECC codeword >> size is not 512 bytes, then the @ecc_bits is 0xff. The host _SHOULD_ >> then > > Section 3.4.2 also says, > "If a value of 0 is reported then this ECC Information Block is > invalid and should not be used". > > Is this taken care anywhere? I did not take care of this case. :( > >> read the Extended ECC information that is part of the extended parameter >> page to retrieve the ECC requirements for this device. >> >> This patch implement the reading of the Extended Parameter Page, and >> parses >> the sections for ECC type, and get the ECC info from the ECC section. >> >> Tested this patch with Micron MT29F64G08CBABAWP. >> >> Acked-by: Pekon Gupta <pekon@ti.com> >> Signed-off-by: Huang Shijie <b32955@freescale.com> >> --- >> drivers/mtd/nand/nand_base.c | 77 >> ++++++++++++++++++++++++++++++++++++++++++ >> 1 files changed, 77 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c >> index 15630ef..e98ea69 100644 >> --- a/drivers/mtd/nand/nand_base.c >> +++ b/drivers/mtd/nand/nand_base.c >> @@ -2835,6 +2835,68 @@ static u16 onfi_crc16(u16 crc, u8 const *p, >> size_t len) >> return crc; >> } >> >> +/* Parse the Extended Parameter Page. */ >> +static int nand_flash_detect_ext_param_page(struct mtd_info *mtd, >> + struct nand_chip *chip, struct nand_onfi_params *p) >> +{ >> + struct onfi_ext_param_page *ep; >> + struct onfi_ext_section *s; >> + struct onfi_ext_ecc_info *ecc; >> + uint8_t *cursor; >> + int len; >> + int ret; >> + int i; >> + >> + len = le16_to_cpu(p->ext_param_page_length) * 16; >> + ep = kmalloc(len, GFP_KERNEL); >> + if (!ep) { >> + ret = -ENOMEM; >> + goto ext_out; > > Just return -ENOMEM. > Why do you free the memory when it is not allocated? (Though it > doesn't cause any harm) For the simplicity of the code. i did so on purpose. it's no use to add new error label for this case. thanks Huang Shijie
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c index 15630ef..e98ea69 100644 --- a/drivers/mtd/nand/nand_base.c +++ b/drivers/mtd/nand/nand_base.c @@ -2835,6 +2835,68 @@ static u16 onfi_crc16(u16 crc, u8 const *p, size_t len) return crc; } +/* Parse the Extended Parameter Page. */ +static int nand_flash_detect_ext_param_page(struct mtd_info *mtd, + struct nand_chip *chip, struct nand_onfi_params *p) +{ + struct onfi_ext_param_page *ep; + struct onfi_ext_section *s; + struct onfi_ext_ecc_info *ecc; + uint8_t *cursor; + int len; + int ret; + int i; + + len = le16_to_cpu(p->ext_param_page_length) * 16; + ep = kmalloc(len, GFP_KERNEL); + if (!ep) { + ret = -ENOMEM; + goto ext_out; + } + + /* Send our own NAND_CMD_PARAM. */ + chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1); + + /* Use the Change Read Column command to skip the ONFI param pages. */ + chip->cmdfunc(mtd, NAND_CMD_RNDOUT, + sizeof(*p) * p->num_of_param_pages , -1); + + /* Read out the Extended Parameter Page. */ + chip->read_buf(mtd, (uint8_t *)ep, len); + if ((onfi_crc16(ONFI_CRC_BASE, ((uint8_t *)ep) + 2, len - 2) + != le16_to_cpu(ep->crc)) || strncmp(ep->sig, "EPPS", 4)) { + pr_debug("fail in the CRC.\n"); + ret = -EINVAL; + goto ext_out; + } + + /* find the ECC section. */ + cursor = (uint8_t *)(ep + 1); + for (i = 0; i < ONFI_EXT_SECTION_MAX; i++) { + s = ep->sections + i; + if (s->type == ONFI_SECTION_TYPE_2) + break; + cursor += s->length * 16; + } + if (i == ONFI_EXT_SECTION_MAX) { + pr_debug("We can not find the ECC section.\n"); + ret = -EINVAL; + goto ext_out; + } + + /* get the info we want. */ + ecc = (struct onfi_ext_ecc_info *)cursor; + chip->ecc_strength = ecc->ecc_bits; + chip->ecc_step = 1 << ecc->codeword_size; + + pr_info("ONFI extended param page detected.\n"); + return 0; + +ext_out: + kfree(ep); + return ret; +} + /* * Check if the NAND chip is ONFI compliant, returns 1 if it is, 0 otherwise. */ @@ -2903,6 +2965,21 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip, if (p->ecc_bits != 0xff) { chip->ecc_strength = p->ecc_bits; chip->ecc_step = 512; + } else if (chip->onfi_version >= 21 && + (onfi_feature(chip) & ONFI_FEATURE_EXT_PARAM_PAGE)) { + + /* + * The nand_flash_detect_ext_param_page() uses the + * Change Read Column command which maybe not supported + * by the chip->cmdfunc. So try to update the chip->cmdfunc + * now. We do not replace user supplied command function. + */ + if (mtd->writesize > 512 && chip->cmdfunc == nand_command) + chip->cmdfunc = nand_command_lp; + + /* The Extended Parameter Page is supported since ONFI 2.1. */ + if (nand_flash_detect_ext_param_page(mtd, chip, p)) + pr_info("Failed to detect the extended param page.\n"); } pr_info("ONFI flash detected\n");