Hi Sascha,
Am 12.06.2017 um 15:41 schrieb Sascha Hauer:
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.
With these changes it works, but I only testes with nand_bitflip. Idk if
there're better ways to test NAND ECC. However, after 6 bitflips on one
subpage it will successfully boot in the backup partition, with less
than 6 it will boot the bitflipped-partition.
Daniel
diff --git a/drivers/mtd/nand/nand_omap_gpmc.c
b/drivers/mtd/nand/nand_omap_gpmc.c
index 2e130bf..9006e2e 100644
--- a/drivers/mtd/nand/nand_omap_gpmc.c
+++ b/drivers/mtd/nand/nand_omap_gpmc.c
@@ -684,10 +684,10 @@ static int omap_gpmc_read_page_bch_rom_mode(struct
mtd_info *mtd,
__omap_calculate_ecc(mtd, buf, ecc_calc, 1);
- 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;
+ for (i = 0; eccsteps; eccsteps--, i += eccbytes, buf += eccsize) {
+ stat = omap_correct_bch(mtd, buf, ecc_code, ecc_calc);
+ ecc_code += eccbytes + 1;
+ ecc_calc += eccbytes;
if (stat < 0) {
mtd->ecc_stats.failed++;
} else {
Sascha
--------------------8<------------------------------
From 2c65a009dbcf2136e037f009b50306aa080e2920 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxx>
---
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;
--
Mit freundlichen Grüßen,
With best regards,
Daniel Schultz
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
