Re: [PATCH v5 06/11] mtd: get the ECC info from the Extended Parameter Page

[Vikram Narayanan <vikram186@gmail.com>]

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