[PATCH] mtd: nand: omap: Fix BCH bit correction
Sascha Hauer
s.hauer at pengutronix.de
Mon Jun 12 06:41:56 PDT 2017
Hi Daniel,
On Fri, Jun 09, 2017 at 03:28:59PM +0200, Daniel Schultz wrote:
> Hi,
>
> Am 09.06.2017 um 11:08 schrieb Sascha Hauer:
> > On Fri, Jun 09, 2017 at 10:17:55AM +0200, Daniel Schultz wrote:
> > > Hi Sascha,
> > >
> > > > >
> > > > > And can not work. Additionally eccsteps must be set to 1 in
> > > > > omap_correct_bch(). This effectively makes the loop in this function
> > > > > unnecessary which can then removed.
> > > >
> > > > Which then means omap_gpmc_read_page_bch_rom_mode() has to iterate over
> > > > ecc.steps itself, just like the other read_page implementations in the
> > > > framework do.
> > > >
> > > So, the previous assignment of eccsteps was fine?
> >
> > I just sent an updated patch(-series). Could you give it a try?
> >
> It works, but the current version only changes the local copy of the
> pointer. As a result of that it will only check the first 512 Bytes.
> I appended a double pointer workaround for this problem :)
The pointer should be incremented by the caller, not by
omap_correct_bch().
>
>
> omap_correct_data() also calls omap_correct_bch(). Does Barebox correct NAND
> partitions? I have never seen this. Maybe we need here also a loop.
The core already loops around eccsteps when calling ecc.correct.
I digged a bit further and this is what I can come up with. Anyway, I am
getting less and less confident that the patch can work.
Please give it a try and see if it works. If it doesn't and we can't
find out what's wrong I tend to take your original patch, although I
still think this is the wrong solution.
Sascha
--------------------8<------------------------------
>From 2c65a009dbcf2136e037f009b50306aa080e2920 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer at pengutronix.de>
Date: Fri, 9 Jun 2017 10:45:21 +0200
Subject: [PATCH] mtd: nand_omap_gpmc: Fix ecc size
The ECC size for BCH correction is always 512 byte. Correct the ecc.size
for the OMAP_ECC_BCH8_CODE_HW mode from 2048 to 512. This change will
let the framework iterate over the 4 ecc steps and we no longer need
special cases in omap_correct_bch().
Signed-off-by: Sascha Hauer <s.hauer at pengutronix.de>
---
drivers/mtd/nand/nand_omap_gpmc.c | 130 +++++++++++++++++---------------------
1 file changed, 57 insertions(+), 73 deletions(-)
diff --git a/drivers/mtd/nand/nand_omap_gpmc.c b/drivers/mtd/nand/nand_omap_gpmc.c
index e18ce6358a..2e130bfd9a 100644
--- a/drivers/mtd/nand/nand_omap_gpmc.c
+++ b/drivers/mtd/nand/nand_omap_gpmc.c
@@ -297,85 +297,59 @@ static int omap_correct_bch(struct mtd_info *mtd, uint8_t *dat,
{
struct nand_chip *nand = (struct nand_chip *)(mtd->priv);
struct gpmc_nand_info *oinfo = (struct gpmc_nand_info *)(nand->priv);
- int i, j, eccflag, totalcount, actual_eccsize;
+ int j, eccflag;
const uint8_t *erased_ecc_vec;
unsigned int err_loc[8];
int bitflip_count;
int bch_max_err;
-
- int eccsteps = (nand->ecc.mode == NAND_ECC_HW) &&
- (nand->ecc.size == 2048) ? 4 : 1;
int eccsize = oinfo->nand.ecc.bytes;
+ bool is_error_reported = false;
- switch (oinfo->ecc_mode) {
- case OMAP_ECC_BCH8_CODE_HW:
- eccsize /= eccsteps;
- actual_eccsize = eccsize;
- erased_ecc_vec = bch8_vector;
- bch_max_err = BCH8_MAX_ERROR;
- break;
- case OMAP_ECC_BCH8_CODE_HW_ROMCODE:
- actual_eccsize = eccsize - 1;
- erased_ecc_vec = bch8_vector;
- bch_max_err = BCH8_MAX_ERROR;
- break;
- default:
- dev_err(oinfo->pdev, "invalid driver configuration\n");
- return -EINVAL;
- }
-
- totalcount = 0;
+ erased_ecc_vec = bch8_vector;
+ bch_max_err = BCH8_MAX_ERROR;
- for (i = 0; i < eccsteps; i++) {
- bool is_error_reported = false;
- bitflip_count = 0;
- eccflag = 0;
+ bitflip_count = 0;
+ eccflag = 0;
- /* check for any ecc error */
- for (j = 0; (j < actual_eccsize) && (eccflag == 0); j++) {
- if (calc_ecc[j] != 0) {
- eccflag = 1;
- break;
- }
- }
-
- if (eccflag == 1) {
- if (memcmp(calc_ecc, erased_ecc_vec, actual_eccsize) == 0) {
- /*
- * calc_ecc[] matches pattern for ECC
- * (all 0xff) so this is definitely
- * an erased-page
- */
- } else {
- bitflip_count = nand_check_erased_ecc_chunk(
- dat, oinfo->nand.ecc.size, read_ecc,
- eccsize, NULL, 0, bch_max_err);
- if (bitflip_count < 0)
- is_error_reported = true;
- }
+ /* check for any ecc error */
+ for (j = 0; (j < eccsize) && (eccflag == 0); j++) {
+ if (calc_ecc[j] != 0) {
+ eccflag = 1;
+ break;
}
+ }
- if (is_error_reported) {
- bitflip_count = omap_gpmc_decode_bch(1,
- calc_ecc, err_loc);
+ if (eccflag == 1) {
+ if (memcmp(calc_ecc, erased_ecc_vec, eccsize) == 0) {
+ /*
+ * calc_ecc[] matches pattern for ECC
+ * (all 0xff) so this is definitely
+ * an erased-page
+ */
+ } else {
+ bitflip_count = nand_check_erased_ecc_chunk(
+ dat, oinfo->nand.ecc.size, read_ecc,
+ eccsize, NULL, 0, bch_max_err);
if (bitflip_count < 0)
- return bitflip_count;
-
- for (j = 0; j < bitflip_count; j++) {
- if (err_loc[j] < 4096)
- dat[err_loc[j] >> 3] ^=
- 1 << (err_loc[j] & 7);
- /* else, not interested to correct ecc */
- }
+ is_error_reported = true;
}
+ }
- totalcount += bitflip_count;
- calc_ecc = calc_ecc + actual_eccsize;
- read_ecc = read_ecc + eccsize;
- dat += 512;
+ if (is_error_reported) {
+ bitflip_count = omap_gpmc_decode_bch(1,
+ calc_ecc, err_loc);
+ if (bitflip_count < 0)
+ return bitflip_count;
+
+ for (j = 0; j < bitflip_count; j++) {
+ if (err_loc[j] < 4096)
+ dat[err_loc[j] >> 3] ^=
+ 1 << (err_loc[j] & 7);
+ /* else, not interested to correct ecc */
+ }
}
- return totalcount;
+ return bitflip_count;
}
static int omap_correct_hamming(struct mtd_info *mtd, uint8_t *dat,
@@ -667,6 +641,10 @@ static int omap_gpmc_read_page_bch_rom_mode(struct mtd_info *mtd,
uint8_t *ecc_code = chip->buffers->ecccode;
uint32_t *eccpos = chip->ecc.layout->eccpos;
int stat, i;
+ unsigned int max_bitflips = 0;
+ int eccsize = chip->ecc.size;
+ int eccbytes = chip->ecc.bytes;
+ int eccsteps = chip->ecc.steps;
writel(GPMC_ECC_SIZE_CONFIG_ECCSIZE1(0) |
GPMC_ECC_SIZE_CONFIG_ECCSIZE0(64),
@@ -706,13 +684,19 @@ static int omap_gpmc_read_page_bch_rom_mode(struct mtd_info *mtd,
__omap_calculate_ecc(mtd, buf, ecc_calc, 1);
- stat = omap_correct_bch(mtd, buf, ecc_code, ecc_calc);
- if (stat < 0)
- mtd->ecc_stats.failed++;
- else
- mtd->ecc_stats.corrected += stat;
+ for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
+ stat = omap_correct_bch(mtd, p, ecc_code, ecc_calc);
+ ecc_code += eccsize + 1;
+ ecc_calc += eccsize;
+ if (stat < 0) {
+ mtd->ecc_stats.failed++;
+ } else {
+ mtd->ecc_stats.corrected += stat;
+ max_bitflips = max_t(unsigned int, max_bitflips, stat);
+ }
+ }
- return 0;
+ return max_bitflips;
}
static int omap_gpmc_eccmode(struct gpmc_nand_info *oinfo,
@@ -765,8 +749,8 @@ static int omap_gpmc_eccmode(struct gpmc_nand_info *oinfo,
offset - omap_oobinfo.eccbytes;
break;
case OMAP_ECC_BCH8_CODE_HW:
- oinfo->nand.ecc.bytes = 13 * 4;
- oinfo->nand.ecc.size = 512 * 4;
+ oinfo->nand.ecc.bytes = 13;
+ oinfo->nand.ecc.size = 512;
oinfo->nand.ecc.strength = BCH8_MAX_ERROR;
omap_oobinfo.oobfree->offset = offset;
omap_oobinfo.oobfree->length = minfo->oobsize -
@@ -776,7 +760,7 @@ static int omap_gpmc_eccmode(struct gpmc_nand_info *oinfo,
omap_oobinfo.eccpos[i] = i + offset;
break;
case OMAP_ECC_BCH8_CODE_HW_ROMCODE:
- oinfo->nand.ecc.bytes = 13 + 1;
+ oinfo->nand.ecc.bytes = 13;
oinfo->nand.ecc.size = 512;
oinfo->nand.ecc.strength = BCH8_MAX_ERROR;
nand->ecc.read_page = omap_gpmc_read_page_bch_rom_mode;
--
2.11.0
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
More information about the barebox
mailing list