| Message ID | 1354013431-13598-1-git-send-email-josh.wu@atmel.com |
|---|---|
| State | Accepted |
| Commit | c0c70d9ee6569150ff6a10f9d3c50c63c917e3a1 |
| Headers | show |
On 11/27/2012 02:50 AM, Josh Wu wrote: > This patch fix pmecc's read_page() to return maximum number of bitflips, 0 if uncorrectable. > > In the commit: 3f91e94f7f511de74c0d2abe08672ccdbdd1961c ("mtd: nand: read_page() returns max_bitflips ()"), > The ecc.read_page() is changed to return the maximum number of bitflips. > And when meet uncorrectable bitflips it needs to return 0. > > See the comment in nand.h: > * @read_page: function to read a page according to the ECC generator > * requirements; returns maximum number of bitflips corrected in > * any single ECC step, 0 if bitflips uncorrectable, -EIO hw error > > Signed-off-by: Josh Wu <josh.wu@atmel.com> > --- > change since v1: > 1. add detail commit message for the fix. > 2. return 0 when meet uncorrectable bitflips according to Mike Dunn's suggestion. Reviewed-by: Mike Dunn <mikedunn@newsguy.com> I see now the pmecc controller patch in the git log. Nice work. Mike
On 11/27/2012 10:59 AM, Mike Dunn wrote: > On 11/27/2012 02:50 AM, Josh Wu wrote: >> This patch fix pmecc's read_page() to return maximum number of bitflips, 0 if uncorrectable. >> >> In the commit: 3f91e94f7f511de74c0d2abe08672ccdbdd1961c ("mtd: nand: read_page() returns max_bitflips ()"), >> The ecc.read_page() is changed to return the maximum number of bitflips. >> And when meet uncorrectable bitflips it needs to return 0. >> >> See the comment in nand.h: >> * @read_page: function to read a page according to the ECC generator >> * requirements; returns maximum number of bitflips corrected in >> * any single ECC step, 0 if bitflips uncorrectable, -EIO hw error >> >> Signed-off-by: Josh Wu <josh.wu@atmel.com> >> --- >> change since v1: >> 1. add detail commit message for the fix. >> 2. return 0 when meet uncorrectable bitflips according to Mike Dunn's suggestion. > > > Reviewed-by: Mike Dunn <mikedunn@newsguy.com> > > I see now the pmecc controller patch in the git log. Nice work. BTW, with such a wide range for ecc strength - up to 24 bits, according to the commit message for the pmecc patch - you may want to think about setting an appropriate bitflip_threshold in the driver. I'm not a nand expert, and I don't know much about the atmel_nand specifically, but I would think that if 23 bits are corrected on a page of size 2k (or less), maybe a return code of -EUCLEAN from mtd_read() might be appropriate. Mike
Hi, Mike On 11/28/2012 3:35 AM, Mike Dunn wrote: > On 11/27/2012 10:59 AM, Mike Dunn wrote: >> On 11/27/2012 02:50 AM, Josh Wu wrote: >>> This patch fix pmecc's read_page() to return maximum number of bitflips, 0 if uncorrectable. >>> >>> In the commit: 3f91e94f7f511de74c0d2abe08672ccdbdd1961c ("mtd: nand: read_page() returns max_bitflips ()"), >>> The ecc.read_page() is changed to return the maximum number of bitflips. >>> And when meet uncorrectable bitflips it needs to return 0. >>> >>> See the comment in nand.h: >>> * @read_page: function to read a page according to the ECC generator >>> * requirements; returns maximum number of bitflips corrected in >>> * any single ECC step, 0 if bitflips uncorrectable, -EIO hw error >>> >>> Signed-off-by: Josh Wu <josh.wu@atmel.com> >>> --- >>> change since v1: >>> 1. add detail commit message for the fix. >>> 2. return 0 when meet uncorrectable bitflips according to Mike Dunn's suggestion. >> >> Reviewed-by: Mike Dunn <mikedunn@newsguy.com> >> >> I see now the pmecc controller patch in the git log. Nice work. > > BTW, with such a wide range for ecc strength - up to 24 bits, according to the > commit message for the pmecc patch - you may want to think about setting an > appropriate bitflip_threshold in the driver. I'm not a nand expert, and I don't > know much about the atmel_nand specifically, but I would think that if 23 bits > are corrected on a page of size 2k (or less), maybe a return code of -EUCLEAN > from mtd_read() might be appropriate. After checking the nand_base.c, I saw it will set the mtd.bitflip_threshold to mtd->ecc.strength during nand_scan_tail(). in the atmel_nand code, the ecc strength will be set correctly, that means bitflip_threashold should be set up correctly by default. so I think I don't need set up the the bitflip_threshold anymore if I set ecc strength correctly. Am I missing any point here? Thanks, Josh Wu > > Mike
On 11/28/2012 11:45 PM, Josh Wu wrote: > Hi, Mike > > On 11/28/2012 3:35 AM, Mike Dunn wrote: >> >> BTW, with such a wide range for ecc strength - up to 24 bits, according to the >> commit message for the pmecc patch - you may want to think about setting an >> appropriate bitflip_threshold in the driver. I'm not a nand expert, and I don't >> know much about the atmel_nand specifically, but I would think that if 23 bits >> are corrected on a page of size 2k (or less), maybe a return code of -EUCLEAN >> from mtd_read() might be appropriate. > > After checking the nand_base.c, I saw it will set the mtd.bitflip_threshold to > mtd->ecc.strength during nand_scan_tail(). > in the atmel_nand code, the ecc strength will be set correctly, that means > bitflip_threashold should be set up correctly by default. > so I think I don't need set up the the bitflip_threshold anymore if I set ecc > strength correctly. Am I missing any point here? No, you are correct; bitflip_threshold is set to ecc.strength by default if the driver has not assigned it a value already. I just wondered if the default value is too liberal given the possibility of such high ecc strength in your case. With bitflip_threshold == 24 (default for the greatest possible ecc.strength on atmel_nand with pmecc) mtd_read() will not return -EUCLEAN until 24 bitflips are corrected on a page that is no greater than 2k in size, and may be as small as 512 bytes (if I read the code correctly). The -EUCLEAN return code is used by higher layers as an indication that a block may be going bad. I don't know whether or not a block bears scrutiny when 23 bitflips are corrected on a page. I guess it depends on the particular flash device. It was just a thought. Hope this makes sense. Mike
On Tue, 2012-11-27 at 18:50 +0800, Josh Wu wrote: > This patch fix pmecc's read_page() to return maximum number of bitflips, 0 if uncorrectable. > > In the commit: 3f91e94f7f511de74c0d2abe08672ccdbdd1961c ("mtd: nand: read_page() returns max_bitflips ()"), > The ecc.read_page() is changed to return the maximum number of bitflips. > And when meet uncorrectable bitflips it needs to return 0. > > See the comment in nand.h: > * @read_page: function to read a page according to the ECC generator > * requirements; returns maximum number of bitflips corrected in > * any single ECC step, 0 if bitflips uncorrectable, -EIO hw error > > Signed-off-by: Josh Wu <josh.wu@atmel.com> Pushed to l2-mtd.git, thanks!
diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c index c918386..1669d27 100644 --- a/drivers/mtd/nand/atmel_nand.c +++ b/drivers/mtd/nand/atmel_nand.c @@ -723,6 +723,7 @@ static int pmecc_correction(struct mtd_info *mtd, u32 pmecc_stat, uint8_t *buf, struct atmel_nand_host *host = nand_chip->priv; int i, err_nbr, eccbytes; uint8_t *buf_pos; + int total_err = 0; eccbytes = nand_chip->ecc.bytes; for (i = 0; i < eccbytes; i++) @@ -750,12 +751,13 @@ normal_check: pmecc_correct_data(mtd, buf_pos, ecc, i, host->pmecc_bytes_per_sector, err_nbr); mtd->ecc_stats.corrected += err_nbr; + total_err += err_nbr; } } pmecc_stat >>= 1; } - return 0; + return total_err; } static int atmel_nand_pmecc_read_page(struct mtd_info *mtd, @@ -767,6 +769,7 @@ static int atmel_nand_pmecc_read_page(struct mtd_info *mtd, uint32_t *eccpos = chip->ecc.layout->eccpos; uint32_t stat; unsigned long end_time; + int bitflips = 0; pmecc_writel(host->ecc, CTRL, PMECC_CTRL_RST); pmecc_writel(host->ecc, CTRL, PMECC_CTRL_DISABLE); @@ -789,11 +792,14 @@ static int atmel_nand_pmecc_read_page(struct mtd_info *mtd, } stat = pmecc_readl_relaxed(host->ecc, ISR); - if (stat != 0) - if (pmecc_correction(mtd, stat, buf, &oob[eccpos[0]]) != 0) - return -EIO; + if (stat != 0) { + bitflips = pmecc_correction(mtd, stat, buf, &oob[eccpos[0]]); + if (bitflips < 0) + /* uncorrectable errors */ + return 0; + } - return 0; + return bitflips; } static int atmel_nand_pmecc_write_page(struct mtd_info *mtd,
This patch fix pmecc's read_page() to return maximum number of bitflips, 0 if uncorrectable. In the commit: 3f91e94f7f511de74c0d2abe08672ccdbdd1961c ("mtd: nand: read_page() returns max_bitflips ()"), The ecc.read_page() is changed to return the maximum number of bitflips. And when meet uncorrectable bitflips it needs to return 0. See the comment in nand.h: * @read_page: function to read a page according to the ECC generator * requirements; returns maximum number of bitflips corrected in * any single ECC step, 0 if bitflips uncorrectable, -EIO hw error Signed-off-by: Josh Wu <josh.wu@atmel.com> --- change since v1: 1. add detail commit message for the fix. 2. return 0 when meet uncorrectable bitflips according to Mike Dunn's suggestion. drivers/mtd/nand/atmel_nand.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)