Messages in this thread | | | From | Masahiro Yamada <> | Date | Fri, 2 Dec 2016 13:26:27 +0900 | Subject | Re: [PATCH 15/39] mtd: nand: denali: improve readability of handle_ecc() |
| |
Hi Boris,
2016-11-28 0:42 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>: >> + if (err_byte < ECC_SECTOR_SIZE) { >> + struct mtd_info *mtd = >> + nand_to_mtd(&denali->nand); >> + int offset; >> + >> + offset = (err_sector * ECC_SECTOR_SIZE + err_byte) * >> + denali->devnum + err_device; >> + /* correct the ECC error */ >> + buf[offset] ^= err_correction_value; >> + mtd->ecc_stats.corrected++; >> + bitflips++; > > Hm, bitflips is what is set in max_bitflips, and apparently the > implementation (which is not yours) is not doing what the core expects. > > You should first count bitflips per sector with something like that: > > bitflips[err_sector]++; > > > And then once you've iterated over all errors do: > > for (i = 0; i < nsectors; i++) > max_bitflips = max(bitflips[err_sector], max_bitflips);
I see.
For soft ECC fixup, we can calculate bitflips for each ECC sector, so I can fix the max_bitflips as the core framework expects.
For hard ECC fixup, the register only reports the number of corrected bit-flips in the whole page (sum from all ECC sectors). We cannot calculate max_bitflips, I think.
BTW, I noticed another problem of the current code.
buf[offset] ^= err_correction_value; mtd->ecc_stats.corrected++; bitflips++;
This code is counting the number of corrected bytes, not the number of corrected bits.
I think multiple bit-flips within one byte can happen.
Perhaps, we should add
hweight8(buf[offset] ^ err_correction_value)
to ecc_stats.corrected and bitflips.
-- Best Regards Masahiro Yamada
| |