lkml.org 
[lkml]   [2013]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v6 05/10] mtd: get the ECC info from the Extended Parameter Page
Hello Huang,

On 5/17/2013 8:47 AM, Huang Shijie wrote:
> Since the ONFI 2.1, the onfi spec adds the Extended Parameter Page
> to store the ECC info.
>
<snip>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index b63b731..0b1162a 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2835,6 +2835,71 @@ 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)
> +{
<snip>
> +
> + /* 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)) {

From section 5.7.2.2.
///Byte 2-5: Extended parameter page signature
This field contains the extended parameter page signature. When two or
more bytes of the signature are valid, then it denotes that a valid copy
of the extended parameter page is present///

But here you are doing a strict check. What if the first two bytes are
valid? Or did I misunderstood something?
If not, I'd prefer to take the strncmp to a separate 'if' so, that you
can do the comparison in the way specified in the ONFI spec.

> + pr_debug("fail in the CRC.\n");

Also, this is the error for Signature failure as well. Please move the
signature comparison to a new if to give better error messages.

> + ret = -EINVAL;
> + goto ext_out;
> + }

Regards,
Vikram


\
 
 \ /
  Last update: 2013-05-17 20:21    [W:0.478 / U:0.196 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site