| Message ID | 1368607232-2210-2-git-send-email-b32955@freescale.com |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2013-05-15 at 16:40 +0800, Huang Shijie wrote: > + * @ecc_strength: [INTERN] ECC correctability from the datasheet. > + * Minimum amount of bit errors per @ecc_step guaranteed to > + * be correctable. If unknown, set to zero. > + * @ecc_step: [INTERN] ECC step required by the @ecc_strength, > + * also from the datasheet. It is the recommended ECC step > + * size, if known; if unknown, set to zero. Here and in other places you talk about "datasheet". Do you assume that the real ECC strength/step used by NAND chips may be different? Or you assume it must be the same?
于 2013年05月15日 20:11, Artem Bityutskiy 写道: > On Wed, 2013-05-15 at 16:40 +0800, Huang Shijie wrote: >> + * @ecc_strength: [INTERN] ECC correctability from the datasheet. >> + * Minimum amount of bit errors per @ecc_step guaranteed to >> + * be correctable. If unknown, set to zero. >> + * @ecc_step: [INTERN] ECC step required by the @ecc_strength, >> + * also from the datasheet. It is the recommended ECC step >> + * size, if known; if unknown, set to zero. > Here and in other places you talk about "datasheet". Do you assume that > the real ECC strength/step used by NAND chips may be different? Or you > assume it must be the same? > The two fields are used to store the ecc info from the datasheet. The two fields are just for a reference. [1] The nand controller may do not use these two fields, it's ok; For example, the datasheet requires "4bits per 512 bytes". The nand controller can uses 8bits per 512 bytes. [2] but sometimes the nand controller must use these two fields. For example, the datasheet requires "40bits per 1024 bytes". For the hardware limit, the nand controller(BCH) may supports the 40bits ecc in the maximum. So the nand controller must use these two fields now. thanks Huang Shijie
On Thu, 2013-05-16 at 10:16 +0800, Huang Shijie wrote: > 于 2013年05月15日 20:11, Artem Bityutskiy 写道: > > On Wed, 2013-05-15 at 16:40 +0800, Huang Shijie wrote: > >> + * @ecc_strength: [INTERN] ECC correctability from the datasheet. > >> + * Minimum amount of bit errors per @ecc_step guaranteed to > >> + * be correctable. If unknown, set to zero. > >> + * @ecc_step: [INTERN] ECC step required by the @ecc_strength, > >> + * also from the datasheet. It is the recommended ECC step > >> + * size, if known; if unknown, set to zero. > > Here and in other places you talk about "datasheet". Do you assume that > > the real ECC strength/step used by NAND chips may be different? Or you > > assume it must be the same? > > > The two fields are used to store the ecc info from the datasheet. > The two fields are just for a reference. > > [1] The nand controller may do not use these two fields, it's ok; > For example, the datasheet requires "4bits per 512 bytes". > The nand controller can uses 8bits per 512 bytes. > > > [2] but sometimes the nand controller must use these two fields. > For example, the datasheet requires "40bits per 1024 bytes". > For the hardware limit, the nand controller(BCH) may supports the > 40bits ecc in the maximum. > So the nand controller must use these two fields now. I wonder if it makes sense to name things so that it is clear form the names whether that is the "theoretical" datasheet values or the real ones. I would prefer to clearly distinguish between them, in names and comments. Thoughts?
于 2013年05月16日 15:14, Artem Bityutskiy 写道: > On Thu, 2013-05-16 at 10:16 +0800, Huang Shijie wrote: >> 于 2013年05月15日 20:11, Artem Bityutskiy 写道: >>> On Wed, 2013-05-15 at 16:40 +0800, Huang Shijie wrote: >>>> + * @ecc_strength: [INTERN] ECC correctability from the datasheet. >>>> + * Minimum amount of bit errors per @ecc_step guaranteed to >>>> + * be correctable. If unknown, set to zero. >>>> + * @ecc_step: [INTERN] ECC step required by the @ecc_strength, >>>> + * also from the datasheet. It is the recommended ECC step >>>> + * size, if known; if unknown, set to zero. >>> Here and in other places you talk about "datasheet". Do you assume that >>> the real ECC strength/step used by NAND chips may be different? Or you >>> assume it must be the same? >>> >> The two fields are used to store the ecc info from the datasheet. >> The two fields are just for a reference. >> >> [1] The nand controller may do not use these two fields, it's ok; >> For example, the datasheet requires "4bits per 512 bytes". >> The nand controller can uses 8bits per 512 bytes. >> >> >> [2] but sometimes the nand controller must use these two fields. >> For example, the datasheet requires "40bits per 1024 bytes". >> For the hardware limit, the nand controller(BCH) may supports the >> 40bits ecc in the maximum. >> So the nand controller must use these two fields now. > I wonder if it makes sense to name things so that it is clear form the > names whether that is the "theoretical" datasheet values or the real > ones. I would prefer to clearly distinguish between them, in names and > comments. Thoughts? > what's about add the "_datasheet" for these two fields? such as ecc_strength__datasheet;ecc_step__datasheet Huang Shijie
On Thu, 2013-05-16 at 16:06 +0800, Huang Shijie wrote: > what's about add the "_datasheet" for these two fields? > such as > > ecc_strength__datasheet;ecc_step__datasheet May be, if these are not too long names. Or ecc_ds_strignth / ecc_ds_step, for example.
于 2013年05月16日 17:36, Artem Bityutskiy 写道: > May be, if these are not too long names. Or ecc_ds_strignth / > ecc_ds_step, for example. I prefer to the ecc_strength_ds/ecc_step_ds. :) Huang Shijie
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 50c5ea0..134d470 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -434,6 +434,12 @@ struct nand_buffers { * bad block marker position; i.e., BBM == 11110111b is * not bad when badblockbits == 7 * @cellinfo: [INTERN] MLC/multichip data from chip ident + * @ecc_strength: [INTERN] ECC correctability from the datasheet. + * Minimum amount of bit errors per @ecc_step guaranteed to + * be correctable. If unknown, set to zero. + * @ecc_step: [INTERN] ECC step required by the @ecc_strength, + * also from the datasheet. It is the recommended ECC step + * size, if known; if unknown, set to zero. * @numchips: [INTERN] number of physical chips * @chipsize: [INTERN] the size of one chip for multichip arrays * @pagemask: [INTERN] page number mask = number of (pages / chip) - 1 @@ -510,6 +516,8 @@ struct nand_chip { unsigned int pagebuf_bitflips; int subpagesize; uint8_t cellinfo; + uint16_t ecc_strength; + uint16_t ecc_step; int badblockpos; int badblockbits;
1.) Why add the ECC information to the nand_chip{} ? Each nand chip has its requirement for the ECC correctability, such as "4bit ECC for each 512Byte" or "40bit ECC for each 1024Byte". This ECC info is very important to the nand controller, such as gpmi. Take the Micron MT29F64G08CBABA for example, its geometry is 8k page size, 744 bytes oob size and it requires 40bit ECC per 1K bytes. If we do not provide the ECC info to the gpmi nand driver, it has to calculate the ECC correctability itself. The gpmi driver will gets the 56bit ECC for per 1K bytes which is beyond its BCH's 40bit ecc capibility. The gpmi will quits in this case. But in actually, the gpmi can supports this nand chip if it can get the right ECC info. 2.) about the new fields. The @ecc_strength stands for the ecc bits needed within the @ecc_step. Both of the new fields should be set which conform to the datasheet. For example: "4bit ECC for each 512Byte" could be: @ecc_strength = 4, @ecc_step = 512. "40bit ECC for each 1024Byte" could be: @ecc_strength = 40, @ecc_step = 1024. 3.) Why do not re-use the @strength and @size in the nand_ecc_ctrl{}? The @strength and @size in nand_ecc_ctrl{} is used by the nand controller driver, while the @ecc_strength and @ecc_step are get from the datasheet. Signed-off-by: Huang Shijie <b32955@freescale.com> --- include/linux/mtd/nand.h | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)