| Message ID | 1368608042-9558-2-git-send-email-b32955@freescale.com |
|---|---|
| State | New, archived |
| Headers | show |
Hello Huang, On 5/15/2013 2:24 PM, Huang Shijie wrote: > If the nand chip provides us the ECC info, we can use it firstly. > The set_geometry_by_ecc_info() will use the ECC info, and > calculate the parameters we need. > > Rename the old code to lagacy_set_geometry() which will takes effect Couple of nitpicks. Thorough out the code and in comments. s/lagacy/legacy/ > when there is no ECC info from the nand chip or we fails in the ECC info case. > > Signed-off-by: Huang Shijie <b32955@freescale.com> > --- > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 128 +++++++++++++++++++++++++++++++- > 1 files changed, 127 insertions(+), 1 deletions(-) > > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > index 25ecfa1..53180da 100644 > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > @@ -112,7 +112,128 @@ static inline bool gpmi_check_ecc(struct gpmi_nand_data *this) > return true; > } > > -int common_nfc_set_geometry(struct gpmi_nand_data *this) > +/* > + * If we can get the ECC information from the nand chip, we do not > + * need to calculate them ourselves. > + * > + * We may have available oob space in this case. > + */ > +static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this) > +{ > + struct bch_geometry *geo = &this->bch_geometry; > + struct mtd_info *mtd = &this->mtd; > + struct nand_chip *chip = mtd->priv; > + struct nand_oobfree *of = gpmi_hw_ecclayout.oobfree; > + unsigned int block_mark_bit_offset; > + > + if (!(chip->ecc_strength > 0 && chip->ecc_step > 0)) > + return false; > + > + switch (chip->ecc_step) { > + case SZ_512: > + geo->gf_len = 13; > + break; > + case SZ_1K: > + geo->gf_len = 14; > + break; > + default: > + dev_err(this->dev, > + "unsupported nand chip. ecc bits : %d, ecc size : %d\n", > + chip->ecc_strength, chip->ecc_step); > + return false; > + } > + geo->ecc_chunk_size = chip->ecc_step; > + geo->ecc_strength = round_up(chip->ecc_strength, 2); > + if (!gpmi_check_ecc(this)) > + return false; > + > + /* Keep the C >= O */ > + if (geo->ecc_chunk_size < mtd->oobsize) { > + dev_err(this->dev, > + "unsupported nand chip. ecc size: %d, oob size : %d\n", > + chip->ecc_step, mtd->oobsize); > + return false; > + } > + > + /* The default value, see comment in the lagacy_set_geometry(). */ > + geo->metadata_size = 10; If this not gonna change, would it be better to keep it as a macro? > + > + geo->ecc_chunk_count = mtd->writesize / geo->ecc_chunk_size; > + > + /* > + * Now, the NAND chip with 2K page(data chunk is 512byte) shows below: > + * > + * | P | > + * |<----------------------------------------------------->| > + * | | > + * | (Block Mark) | > + * | P' | | | | > + * |<-------------------------------------------->| D | | O' | > + * | |<---->| |<--->| > + * V V V V V > + * +---+----------+-+----------+-+----------+-+----------+-+-----+ > + * | M | data |E| data |E| data |E| data |E| | > + * +---+----------+-+----------+-+----------+-+----------+-+-----+ > + * > + * P : the page size for BCH module. > + * E : The ECC strength. > + * G : the length of Galois Field. > + * N : The chunk count of per page. > + * M : the metasize of per page. > + * C : the ecc chunk size, aka the "data" above. > + * P': the nand chip's page size. > + * O : the nand chip's oob size. I am not sure if my eyes have gone bad. But I couldn't spot the 'O' in the above NAND page diagram. I think 's/D/O'. Regards, Vikram
于 2013年05月16日 00:31, Vikram Narayanan 写道: > Hello Huang, > > On 5/15/2013 2:24 PM, Huang Shijie wrote: >> If the nand chip provides us the ECC info, we can use it firstly. >> The set_geometry_by_ecc_info() will use the ECC info, and >> calculate the parameters we need. >> >> Rename the old code to lagacy_set_geometry() which will takes effect > > Couple of nitpicks. > Thorough out the code and in comments. > s/lagacy/legacy/ > thanks. :) >> when there is no ECC info from the nand chip or we fails in the ECC >> info case. >> >> Signed-off-by: Huang Shijie <b32955@freescale.com> >> --- >> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 128 >> +++++++++++++++++++++++++++++++- >> 1 files changed, 127 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> index 25ecfa1..53180da 100644 >> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> @@ -112,7 +112,128 @@ static inline bool gpmi_check_ecc(struct >> gpmi_nand_data *this) >> return true; >> } >> >> -int common_nfc_set_geometry(struct gpmi_nand_data *this) >> +/* >> + * If we can get the ECC information from the nand chip, we do not >> + * need to calculate them ourselves. >> + * >> + * We may have available oob space in this case. >> + */ >> +static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this) >> +{ >> + struct bch_geometry *geo = &this->bch_geometry; >> + struct mtd_info *mtd = &this->mtd; >> + struct nand_chip *chip = mtd->priv; >> + struct nand_oobfree *of = gpmi_hw_ecclayout.oobfree; >> + unsigned int block_mark_bit_offset; >> + >> + if (!(chip->ecc_strength > 0 && chip->ecc_step > 0)) >> + return false; >> + >> + switch (chip->ecc_step) { >> + case SZ_512: >> + geo->gf_len = 13; >> + break; >> + case SZ_1K: >> + geo->gf_len = 14; >> + break; >> + default: >> + dev_err(this->dev, >> + "unsupported nand chip. ecc bits : %d, ecc size : %d\n", >> + chip->ecc_strength, chip->ecc_step); >> + return false; >> + } >> + geo->ecc_chunk_size = chip->ecc_step; >> + geo->ecc_strength = round_up(chip->ecc_strength, 2); >> + if (!gpmi_check_ecc(this)) >> + return false; >> + >> + /* Keep the C >= O */ >> + if (geo->ecc_chunk_size < mtd->oobsize) { >> + dev_err(this->dev, >> + "unsupported nand chip. ecc size: %d, oob size : %d\n", >> + chip->ecc_step, mtd->oobsize); >> + return false; >> + } >> + >> + /* The default value, see comment in the lagacy_set_geometry(). */ >> + geo->metadata_size = 10; > > If this not gonna change, would it be better to keep it as a macro? > in actually, it will not changed, the ROM may fixes this value to 10. I can submit an extra patch to add a macro to fix it. but could we keep it as it is in this patch set? >> + >> + geo->ecc_chunk_count = mtd->writesize / geo->ecc_chunk_size; >> + >> + /* >> + * Now, the NAND chip with 2K page(data chunk is 512byte) shows below: >> + * >> + * | P | >> + * |<----------------------------------------------------->| >> + * | | >> + * | (Block Mark) | >> + * | P' | | | | >> + * |<-------------------------------------------->| D | | O' | >> + * | |<---->| |<--->| >> + * V V V V V >> + * +---+----------+-+----------+-+----------+-+----------+-+-----+ >> + * | M | data |E| data |E| data |E| data |E| | >> + * +---+----------+-+----------+-+----------+-+----------+-+-----+ >> + * >> + * P : the page size for BCH module. >> + * E : The ECC strength. >> + * G : the length of Galois Field. >> + * N : The chunk count of per page. >> + * M : the metasize of per page. >> + * C : the ecc chunk size, aka the "data" above. >> + * P': the nand chip's page size. >> + * O : the nand chip's oob size. > > I am not sure if my eyes have gone bad. But I couldn't spot the 'O' in > the above NAND page diagram. > I think 's/D/O'. I do not place the "O" in the diagram. fix it in later patch version. thanks Huang Shijie
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c index 25ecfa1..53180da 100644 --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c @@ -112,7 +112,128 @@ static inline bool gpmi_check_ecc(struct gpmi_nand_data *this) return true; } -int common_nfc_set_geometry(struct gpmi_nand_data *this) +/* + * If we can get the ECC information from the nand chip, we do not + * need to calculate them ourselves. + * + * We may have available oob space in this case. + */ +static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this) +{ + struct bch_geometry *geo = &this->bch_geometry; + struct mtd_info *mtd = &this->mtd; + struct nand_chip *chip = mtd->priv; + struct nand_oobfree *of = gpmi_hw_ecclayout.oobfree; + unsigned int block_mark_bit_offset; + + if (!(chip->ecc_strength > 0 && chip->ecc_step > 0)) + return false; + + switch (chip->ecc_step) { + case SZ_512: + geo->gf_len = 13; + break; + case SZ_1K: + geo->gf_len = 14; + break; + default: + dev_err(this->dev, + "unsupported nand chip. ecc bits : %d, ecc size : %d\n", + chip->ecc_strength, chip->ecc_step); + return false; + } + geo->ecc_chunk_size = chip->ecc_step; + geo->ecc_strength = round_up(chip->ecc_strength, 2); + if (!gpmi_check_ecc(this)) + return false; + + /* Keep the C >= O */ + if (geo->ecc_chunk_size < mtd->oobsize) { + dev_err(this->dev, + "unsupported nand chip. ecc size: %d, oob size : %d\n", + chip->ecc_step, mtd->oobsize); + return false; + } + + /* The default value, see comment in the lagacy_set_geometry(). */ + geo->metadata_size = 10; + + geo->ecc_chunk_count = mtd->writesize / geo->ecc_chunk_size; + + /* + * Now, the NAND chip with 2K page(data chunk is 512byte) shows below: + * + * | P | + * |<----------------------------------------------------->| + * | | + * | (Block Mark) | + * | P' | | | | + * |<-------------------------------------------->| D | | O' | + * | |<---->| |<--->| + * V V V V V + * +---+----------+-+----------+-+----------+-+----------+-+-----+ + * | M | data |E| data |E| data |E| data |E| | + * +---+----------+-+----------+-+----------+-+----------+-+-----+ + * + * P : the page size for BCH module. + * E : The ECC strength. + * G : the length of Galois Field. + * N : The chunk count of per page. + * M : the metasize of per page. + * C : the ecc chunk size, aka the "data" above. + * P': the nand chip's page size. + * O : the nand chip's oob size. + * O': the free oob. + * + * The formula for P is : + * + * E * G * N + * P = ------------ + P' + M + * 8 + * + * The position of block mark moves forward in the ECC-based view + * of page, and the delta is: + * + * E * G * (N - 1) + * D = (---------------- + M) + * 8 + * + * Please see the comment in lagacy_set_geometry(). + * With the condition C >= O , we still can get same result. + * So the bit position of the physical block mark within the ECC-based + * view of the page is : + * (P' - D) * 8 + */ + geo->page_size = mtd->writesize + geo->metadata_size + + (geo->gf_len * geo->ecc_strength * geo->ecc_chunk_count) / 8; + + /* The available oob size we have. */ + if (geo->page_size < mtd->writesize + mtd->oobsize) { + of->offset = geo->page_size - mtd->writesize; + of->length = mtd->oobsize - of->offset; + mtd->oobavail = gpmi_hw_ecclayout.oobavail = of->length; + } + + geo->payload_size = mtd->writesize; + + geo->auxiliary_status_offset = ALIGN(geo->metadata_size, 4); + geo->auxiliary_size = ALIGN(geo->metadata_size, 4) + + ALIGN(geo->ecc_chunk_count, 4); + + if (!this->swap_block_mark) + return true; + + /* For bit swap. */ + block_mark_bit_offset = mtd->writesize * 8 - + (geo->ecc_strength * geo->gf_len * (geo->ecc_chunk_count - 1) + + geo->metadata_size * 8); + + geo->block_mark_byte_offset = block_mark_bit_offset / 8; + geo->block_mark_bit_offset = block_mark_bit_offset % 8; + return true; +} + +static int lagacy_set_geometry(struct gpmi_nand_data *this) { struct bch_geometry *geo = &this->bch_geometry; struct mtd_info *mtd = &this->mtd; @@ -224,6 +345,11 @@ int common_nfc_set_geometry(struct gpmi_nand_data *this) return 0; } +int common_nfc_set_geometry(struct gpmi_nand_data *this) +{ + return set_geometry_by_ecc_info(this) ? 0 : lagacy_set_geometry(this); +} + struct dma_chan *get_dma_chan(struct gpmi_nand_data *this) { int chipnr = this->current_chip;
If the nand chip provides us the ECC info, we can use it firstly. The set_geometry_by_ecc_info() will use the ECC info, and calculate the parameters we need. Rename the old code to lagacy_set_geometry() which will takes effect when there is no ECC info from the nand chip or we fails in the ECC info case. Signed-off-by: Huang Shijie <b32955@freescale.com> --- drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 128 +++++++++++++++++++++++++++++++- 1 files changed, 127 insertions(+), 1 deletions(-)