| Message ID | 50BD01F8.3040003@itwatchdogs.com |
|---|---|
| State | New, archived |
| Headers | show |
于 2012年12月04日 03:48, Zach Sadecki 写道: > Always report corrected and failed ECC stats back up to the MTD layer. > Also > return max_bitflips from read_page() as is expected from NAND drivers > now. > > Signed-off-by: Zach Sadecki <zsadecki@itwatchdogs.com> > --- > drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 27 ++++++++------------------- > 1 file changed, 8 insertions(+), 19 deletions(-) > > diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > index d79696b..fe10c53 100644 > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c > @@ -914,8 +914,7 @@ static int gpmi_ecc_read_page(struct mtd_info > *mtd, struct nand_chip *chip, > dma_addr_t auxiliary_phys; > unsigned int i; > unsigned char *status; > - unsigned int failed; > - unsigned int corrected; > + unsigned int max_bitflips = 0; > int ret; > pr_debug("page number is : %d\n", page); > @@ -939,35 +938,25 @@ static int gpmi_ecc_read_page(struct mtd_info > *mtd, struct nand_chip *chip, > payload_virt, payload_phys); > if (ret) { > pr_err("Error in ECC-based read: %d\n", ret); > - goto exit_nfc; > + return ret; > } > /* handle the block mark swapping */ > block_mark_swapping(this, payload_virt, auxiliary_virt); > /* Loop over status bytes, accumulating ECC status. */ > - failed = 0; > - corrected = 0; > - status = auxiliary_virt + nfc_geo->auxiliary_status_offset; > + status = auxiliary_virt + nfc_geo->auxiliary_status_offset; > for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) { > if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED)) > continue; > if (*status == STATUS_UNCORRECTABLE) { > - failed++; > + mtd->ecc_stats.failed++; > continue; > } > - corrected += *status; > - } > - > - /* > - * Propagate ECC status to the owning MTD only when failed or > - * corrected times nearly reaches our ECC correction threshold. > - */ > - if (failed || corrected >= (nfc_geo->ecc_strength - 1)) { > - mtd->ecc_stats.failed += failed; > - mtd->ecc_stats.corrected += corrected; > + mtd->ecc_stats.corrected += *status; > + max_bitflips = max_t(unsigned int, max_bitflips, *status); Does the max_bitflips stand for the whole page's bitfips ? or just the bitfips of one part of the page? The gpmi driver splits a page into several parts when it does the ECC. thanks Huang Shijie > } > if (oob_required) { > @@ -989,8 +978,8 @@ static int gpmi_ecc_read_page(struct mtd_info > *mtd, struct nand_chip *chip, > this->payload_virt, this->payload_phys, > nfc_geo->payload_size, > payload_virt, payload_phys); > -exit_nfc: > - return ret; > + > + return max_bitflips; > } > static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip > *chip,
On 12/03/2012 09:34 PM, Huang Shijie wrote: > 于 2012年12月04日 03:48, Zach Sadecki 写道: >> Always report corrected and failed ECC stats back up to the MTD >> layer. Also >> return max_bitflips from read_page() as is expected from NAND drivers >> now. >> >> Signed-off-by: Zach Sadecki <zsadecki@itwatchdogs.com> >> --- >> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 27 ++++++++------------------- >> 1 file changed, 8 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> index d79696b..fe10c53 100644 >> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c >> @@ -914,8 +914,7 @@ static int gpmi_ecc_read_page(struct mtd_info >> *mtd, struct nand_chip *chip, >> dma_addr_t auxiliary_phys; >> unsigned int i; >> unsigned char *status; >> - unsigned int failed; >> - unsigned int corrected; >> + unsigned int max_bitflips = 0; >> int ret; >> pr_debug("page number is : %d\n", page); >> @@ -939,35 +938,25 @@ static int gpmi_ecc_read_page(struct mtd_info >> *mtd, struct nand_chip *chip, >> payload_virt, payload_phys); >> if (ret) { >> pr_err("Error in ECC-based read: %d\n", ret); >> - goto exit_nfc; >> + return ret; >> } >> /* handle the block mark swapping */ >> block_mark_swapping(this, payload_virt, auxiliary_virt); >> /* Loop over status bytes, accumulating ECC status. */ >> - failed = 0; >> - corrected = 0; >> - status = auxiliary_virt + nfc_geo->auxiliary_status_offset; >> + status = auxiliary_virt + nfc_geo->auxiliary_status_offset; >> for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) { >> if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED)) >> continue; >> if (*status == STATUS_UNCORRECTABLE) { >> - failed++; >> + mtd->ecc_stats.failed++; >> continue; >> } >> - corrected += *status; >> - } >> - >> - /* >> - * Propagate ECC status to the owning MTD only when failed or >> - * corrected times nearly reaches our ECC correction threshold. >> - */ >> - if (failed || corrected >= (nfc_geo->ecc_strength - 1)) { >> - mtd->ecc_stats.failed += failed; >> - mtd->ecc_stats.corrected += corrected; >> + mtd->ecc_stats.corrected += *status; >> + max_bitflips = max_t(unsigned int, max_bitflips, *status); > > Does the max_bitflips stand for the whole page's bitfips ? or just the > bitfips of one part of the page? > The gpmi driver splits a page into several parts when it does the ECC. > > thanks > Huang Shijie > max_bitflips represents the maximum number of bitflips per ECC corrected region in a page. The MTD layer needs it to compare to bitflip_threshold to know when to return -EUCLEAN to upper layers (like UBI) so a nearly corrupt page can be copied elsewhere. This is pretty much identical to the way other drivers do this as I copied it from those. Here's where it was added in the other drivers: http://git.infradead.org/linux-mtd.git/commit/3f91e94f7f511de74c0d2abe08672ccdbdd1961c Zach > >> } >> if (oob_required) { >> @@ -989,8 +978,8 @@ static int gpmi_ecc_read_page(struct mtd_info >> *mtd, struct nand_chip *chip, >> this->payload_virt, this->payload_phys, >> nfc_geo->payload_size, >> payload_virt, payload_phys); >> -exit_nfc: >> - return ret; >> + >> + return max_bitflips; >> } >> static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip >> *chip, > >
于 2012年12月04日 22:40, Zach Sadecki 写道: > max_bitflips represents the maximum number of bitflips per ECC > corrected region in a page. got it. I also tested this patch in the mx6q-arm2 board. it runned well. thanks. Acked-by: Huang Shijie <b32955@freescale.com> Huang Shijie
On Mon, 2012-12-03 at 13:48 -0600, Zach Sadecki wrote: > Always report corrected and failed ECC stats back up to the MTD layer. Also > return max_bitflips from read_page() as is expected from NAND drivers now. > > Signed-off-by: Zach Sadecki <zsadecki@itwatchdogs.com> Could you please send a version which applies cleanly to the l2-mtd.git tree? Thanks! git://git.infradead.org/users/dedekind/l2-mtd.git
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c index d79696b..fe10c53 100644 --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c @@ -914,8 +914,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip, dma_addr_t auxiliary_phys; unsigned int i; unsigned char *status; - unsigned int failed; - unsigned int corrected; + unsigned int max_bitflips = 0; int ret; pr_debug("page number is : %d\n", page); @@ -939,35 +938,25 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip, payload_virt, payload_phys); if (ret) { pr_err("Error in ECC-based read: %d\n", ret); - goto exit_nfc; + return ret; } /* handle the block mark swapping */ block_mark_swapping(this, payload_virt, auxiliary_virt); /* Loop over status bytes, accumulating ECC status. */ - failed = 0; - corrected = 0; - status = auxiliary_virt + nfc_geo->auxiliary_status_offset; + status = auxiliary_virt + nfc_geo->auxiliary_status_offset; for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) { if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED)) continue; if (*status == STATUS_UNCORRECTABLE) { - failed++; + mtd->ecc_stats.failed++; continue; } - corrected += *status; - } - - /* - * Propagate ECC status to the owning MTD only when failed or - * corrected times nearly reaches our ECC correction threshold. - */ - if (failed || corrected >= (nfc_geo->ecc_strength - 1)) { - mtd->ecc_stats.failed += failed; - mtd->ecc_stats.corrected += corrected; + mtd->ecc_stats.corrected += *status; + max_bitflips = max_t(unsigned int, max_bitflips, *status); } if (oob_required) { @@ -989,8 +978,8 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip, this->payload_virt, this->payload_phys, nfc_geo->payload_size, payload_virt, payload_phys); -exit_nfc: - return ret; + + return max_bitflips; } static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
Always report corrected and failed ECC stats back up to the MTD layer. Also return max_bitflips from read_page() as is expected from NAND drivers now. Signed-off-by: Zach Sadecki <zsadecki@itwatchdogs.com> --- drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-)