lkml.org 
[lkml]   [2013]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 3/8] mtd: get the ECC info from the Extended Parameter Page
于 2013年04月23日 05:22, Brian Norris 写道:
> On Mon, Apr 22, 2013 at 12:47 AM, Huang Shijie<b32955@freescale.com> 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
>> 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.
>>
>> Signed-off-by: Huang Shijie<b32955@freescale.com>
>> ---
>> drivers/mtd/nand/nand_base.c | 54 ++++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 54 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index beff911..48ff097 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -2846,6 +2846,56 @@ static u16 onfi_crc16(u16 crc, u8 const *p, size_t len)
>> return crc;
>> }
>>
>> +/* Parse the Extended Parameter Page. */
>> +static void nand_flash_detect_ext_param_page(struct mtd_info *mtd,
>> + struct nand_chip *chip, struct nand_onfi_params *p, int last)
>> +{
> I think we want a return code (int) for this function. It obviously
> can fail, and the caller needs to know this.
I not sure who will uses ecc_strength/ecc_size, except the gpmi.
So i ignore the return value.

If you think we should add it, i will add it.
> The "last" parameter is not very obvious until you read the whole
> function, where you see that this function assumes a lot about the
> caller. Please address the comments below and/or fully document the
> parameters and calling context for this function.
>
ok. I can add more comments.

>> + struct onfi_ext_param_page *ep;
>> + struct onfi_ext_section *s;
>> + struct onfi_ext_ecc_info *ecc;
>> + uint8_t *cursor;
>> + int len;
>> + int i;
>> +
>> + len = le16_to_cpu(p->ext_param_page_length) * 16;
>> + ep = kcalloc(1, max_t(int, len, sizeof(*p)), GFP_KERNEL);
>> + if (!ep)
>> + goto ext_out;
>> +
>> + /*
>> + * Skip the ONFI Parameter Pages.
>> + * The Change Read Columm command may does not works here.
> Why not?
>
You can give me a fix patch which bases on my patch set.
I can test it. :)

I tried to use the Change-read-columm command, but failed.

>> + */
>> + for (i = last + 1; i< p->num_of_param_pages; i++)
>> + chip->read_buf(mtd, (uint8_t *)ep, sizeof(*p));
> You never sent a command to the chip. How can you expect to read from it?
we have sent a command in the nand_flash_detect_onfi().
> It seems that you are writing this function with the assumption of a
> particular calling context (a context in which the last command was
> CMD_PARAM). IMO, it would make a lot more sense that this function
> actually send its own CMD_PARAM followed by either X bytes of skipped
> read_buf() or a change read column command. Then it doesn't need the
> "last" argument, and its usage makes much more sense.
>
I added the "last" argument just because the Change-read-column command
did not works.


thanks
Huang Shijie


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

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