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

[Huang Shijie <b32955@freescale.com>]

于 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