| Message ID | 1378797908-22567-3-git-send-email-pekon@ti.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Scott Wood |
| Headers | show |
On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote: > With increase in NAND flash densities occurence of bit-flips has increased. > Thus stronger ECC schemes are required for detecting and correcting multiple > simultaneous bit-flips in same NAND page. But stronger ECC schemes have large > ECC syndrome which require more space in OOB/Spare. > This patch add support for BCH16_ECC: > (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data. > (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B. > > Due to (b) this scheme can only be used with NAND devices which have enough > OOB to satisfy following equation: > OOBsize per page >= 26 * (page-size / 512) > > Signed-off-by: Pekon Gupta <pekon@ti.com> > > --- > arch/arm/include/asm/arch-am33xx/cpu.h | 15 ++++- > arch/arm/include/asm/arch-am33xx/omap_gpmc.h | 4 +- > drivers/mtd/nand/omap_gpmc.c | 87 +++++++++++++++++++++++----- > include/mtd/mtd-abi.h | 3 +- > 4 files changed, 90 insertions(+), 19 deletions(-) This doesn't apply cleanly. > diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h b/arch/arm/include/asm/arch-am33xx/cpu.h > index 10b56e0..1de92e6 100644 > --- a/arch/arm/include/asm/arch-am33xx/cpu.h > +++ b/arch/arm/include/asm/arch-am33xx/cpu.h > @@ -63,7 +63,16 @@ struct gpmc_cs { > }; > > struct bch_res_0_3 { > - u32 bch_result_x[4]; > + u32 bch_result0; > + u32 bch_result1; > + u32 bch_result2; > + u32 bch_result3; > +}; Is this really an improvement? It would also be nice if headers for things in drivers/mtd/nand weren't in arch. -Scott
> From: Scott Wood [mailto:scottwood@freescale.com] > > On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote: > > With increase in NAND flash densities occurence of bit-flips has increased. > > Thus stronger ECC schemes are required for detecting and correcting > multiple > > simultaneous bit-flips in same NAND page. But stronger ECC schemes have > large > > ECC syndrome which require more space in OOB/Spare. > > This patch add support for BCH16_ECC: > > (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data. > > (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B. > > > > Due to (b) this scheme can only be used with NAND devices which have > enough > > OOB to satisfy following equation: > > OOBsize per page >= 26 * (page-size / 512) > > > > Signed-off-by: Pekon Gupta <pekon@ti.com> > > > > --- > > arch/arm/include/asm/arch-am33xx/cpu.h | 15 ++++- > > arch/arm/include/asm/arch-am33xx/omap_gpmc.h | 4 +- > > drivers/mtd/nand/omap_gpmc.c | 87 > +++++++++++++++++++++++----- > > include/mtd/mtd-abi.h | 3 +- > > 4 files changed, 90 insertions(+), 19 deletions(-) > > This doesn't apply cleanly. > This one is on top of previous patch series like.. [Part 1] http://lists.denx.de/pipermail/u-boot/2013-November/167393.html [Part 2] http://lists.denx.de/pipermail/u-boot/2013-November/167445.html So once the above series are cleaned and accepted. I'll rebase and re-send this series for BCH16 ecc-scheme again.. > > diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h > b/arch/arm/include/asm/arch-am33xx/cpu.h > > index 10b56e0..1de92e6 100644 > > --- a/arch/arm/include/asm/arch-am33xx/cpu.h > > +++ b/arch/arm/include/asm/arch-am33xx/cpu.h > > @@ -63,7 +63,16 @@ struct gpmc_cs { > > }; > > > > struct bch_res_0_3 { > > - u32 bch_result_x[4]; > > + u32 bch_result0; > > + u32 bch_result1; > > + u32 bch_result2; > > + u32 bch_result3; > > +}; > > Is this really an improvement? > Good for readability to match it to actual IP spec.. Each of bch_resultx map to individual 32-bit registers named accordingly in IP spec. > It would also be nice if headers for things in drivers/mtd/nand weren't > in arch. > I agree.. but I would take that separately, as I need to test all other platforms also for this change .. (like am335x, am43xx, omap3.. ) with regards, pekon
On Tue, 2013-11-19 at 13:21 +0000, Gupta, Pekon wrote: > > From: Scott Wood [mailto:scottwood@freescale.com] > > > On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote: > > > With increase in NAND flash densities occurence of bit-flips has increased. > > > Thus stronger ECC schemes are required for detecting and correcting > > multiple > > > simultaneous bit-flips in same NAND page. But stronger ECC schemes have > > large > > > ECC syndrome which require more space in OOB/Spare. > > > This patch add support for BCH16_ECC: > > > (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data. > > > (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B. > > > > > > Due to (b) this scheme can only be used with NAND devices which have > > enough > > > OOB to satisfy following equation: > > > OOBsize per page >= 26 * (page-size / 512) > > > > > > Signed-off-by: Pekon Gupta <pekon@ti.com> > > > > > > --- > > > arch/arm/include/asm/arch-am33xx/cpu.h | 15 ++++- > > > arch/arm/include/asm/arch-am33xx/omap_gpmc.h | 4 +- > > > drivers/mtd/nand/omap_gpmc.c | 87 > > +++++++++++++++++++++++----- > > > include/mtd/mtd-abi.h | 3 +- > > > 4 files changed, 90 insertions(+), 19 deletions(-) > > > > This doesn't apply cleanly. > > > This one is on top of previous patch series like.. > [Part 1] http://lists.denx.de/pipermail/u-boot/2013-November/167393.html > [Part 2] http://lists.denx.de/pipermail/u-boot/2013-November/167445.html > > So once the above series are cleaned and accepted. I'll rebase and re-send this > series for BCH16 ecc-scheme again.. > > > > > diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h > > b/arch/arm/include/asm/arch-am33xx/cpu.h > > > index 10b56e0..1de92e6 100644 > > > --- a/arch/arm/include/asm/arch-am33xx/cpu.h > > > +++ b/arch/arm/include/asm/arch-am33xx/cpu.h > > > @@ -63,7 +63,16 @@ struct gpmc_cs { > > > }; > > > > > > struct bch_res_0_3 { > > > - u32 bch_result_x[4]; > > > + u32 bch_result0; > > > + u32 bch_result1; > > > + u32 bch_result2; > > > + u32 bch_result3; > > > +}; > > > > Is this really an improvement? > > > Good for readability to match it to actual IP spec.. > Each of bch_resultx map to individual 32-bit registers named accordingly > in IP spec. Hardware often names things "FOO1, FOO2, etc", but in C it's often better to express as foo[n] rather than foo1, foo2, etc. > > It would also be nice if headers for things in drivers/mtd/nand weren't > > in arch. > > > I agree.. but I would take that separately, as I need to test all other > platforms also for this change .. (like am335x, am43xx, omap3.. ) Yes, of course it would be separate. -Scott
On Tue, 2013-11-19 at 13:21 +0000, Gupta, Pekon wrote: > > From: Scott Wood [mailto:scottwood@freescale.com] > > > On Tue, Sep 10, 2013 at 12:55:07PM +0530, pekon gupta wrote: > > > With increase in NAND flash densities occurence of bit-flips has increased. > > > Thus stronger ECC schemes are required for detecting and correcting > > multiple > > > simultaneous bit-flips in same NAND page. But stronger ECC schemes have > > large > > > ECC syndrome which require more space in OOB/Spare. > > > This patch add support for BCH16_ECC: > > > (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data. > > > (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B. > > > > > > Due to (b) this scheme can only be used with NAND devices which have > > enough > > > OOB to satisfy following equation: > > > OOBsize per page >= 26 * (page-size / 512) > > > > > > Signed-off-by: Pekon Gupta <pekon@ti.com> > > > > > > --- > > > arch/arm/include/asm/arch-am33xx/cpu.h | 15 ++++- > > > arch/arm/include/asm/arch-am33xx/omap_gpmc.h | 4 +- > > > drivers/mtd/nand/omap_gpmc.c | 87 > > +++++++++++++++++++++++----- > > > include/mtd/mtd-abi.h | 3 +- > > > 4 files changed, 90 insertions(+), 19 deletions(-) > > > > This doesn't apply cleanly. > > > This one is on top of previous patch series like.. > [Part 1] http://lists.denx.de/pipermail/u-boot/2013-November/167393.html > [Part 2] http://lists.denx.de/pipermail/u-boot/2013-November/167445.html > > So once the above series are cleaned and accepted. I'll rebase and re-send this > series for BCH16 ecc-scheme again.. Please be sure to mention such dependencies in the future. -Scott
diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h b/arch/arm/include/asm/arch-am33xx/cpu.h index 10b56e0..1de92e6 100644 --- a/arch/arm/include/asm/arch-am33xx/cpu.h +++ b/arch/arm/include/asm/arch-am33xx/cpu.h @@ -63,7 +63,16 @@ struct gpmc_cs { }; struct bch_res_0_3 { - u32 bch_result_x[4]; + u32 bch_result0; + u32 bch_result1; + u32 bch_result2; + u32 bch_result3; +}; + +struct bch_res_4_6 { + u32 bch_result4; + u32 bch_result5; + u32 bch_result6; }; struct gpmc { @@ -95,7 +104,9 @@ struct gpmc { u8 res7[12]; /* 0x224 */ u32 testmomde_ctrl; /* 0x230 */ u8 res8[12]; /* 0x234 */ - struct bch_res_0_3 bch_result_0_3[2]; /* 0x240 */ + struct bch_res_0_3 bch_result_0_3[8]; /* 0x240 - 0x2BF */ + u8 res9[16 * 4]; /* 0x2C0 - 0x2FF */ + struct bch_res_4_6 bch_result_4_6[8]; /* 0x300 - 0x37F */ }; /* Used for board specific gpmc initialization */ diff --git a/arch/arm/include/asm/arch-am33xx/omap_gpmc.h b/arch/arm/include/asm/arch-am33xx/omap_gpmc.h index 69982c1..ade0ca4 100644 --- a/arch/arm/include/asm/arch-am33xx/omap_gpmc.h +++ b/arch/arm/include/asm/arch-am33xx/omap_gpmc.h @@ -16,7 +16,9 @@ enum omap_ecc { /* 8-bit ECC calculation by GPMC, Error detection by Software */ OMAP_ECC_BCH8_CODE_HW_DETECTION_SW, /* 8-bit ECC calculation by GPMC, Error detection by ELM */ - OMAP_ECC_BCH8_CODE_HW + OMAP_ECC_BCH8_CODE_HW, + /* 16-bit ECC calculation by GPMC, Error detection by ELM */ + OMAP_ECC_BCH16_CODE_HW, }; #endif /* __ASM_ARCH_OMAP_GPMC_H */ diff --git a/drivers/mtd/nand/omap_gpmc.c b/drivers/mtd/nand/omap_gpmc.c index c1a7317..b044f00 100644 --- a/drivers/mtd/nand/omap_gpmc.c +++ b/drivers/mtd/nand/omap_gpmc.c @@ -154,21 +154,10 @@ static int __maybe_unused omap_correct_data(struct mtd_info *mtd, uint8_t *dat, struct nand_bch_priv { uint8_t mode; uint8_t type; - uint8_t nibbles; struct bch_control *control; enum omap_ecc ecc_scheme; }; -/* bch types */ -#define ECC_BCH4 0 -#define ECC_BCH8 1 -#define ECC_BCH16 2 - -/* BCH nibbles for diff bch levels */ -#define ECC_BCH4_NIBBLES 13 -#define ECC_BCH8_NIBBLES 26 -#define ECC_BCH16_NIBBLES 52 - /* * This can be a single instance cause all current users have only one NAND * with nearly the same setup (BCH8, some with ELM and others with sw BCH @@ -177,7 +166,6 @@ struct nand_bch_priv { */ static __maybe_unused struct nand_bch_priv bch_priv = { .type = ECC_BCH8, - .nibbles = ECC_BCH8_NIBBLES, .control = NULL }; @@ -243,6 +231,19 @@ static void omap_enable_hwecc(struct mtd_info *mtd, int32_t mode) eccsize1 = 2; /* non-ECC bits in nibbles per sector */ } break; + case OMAP_ECC_BCH16_CODE_HW: + ecc_algo = 0x1; + bch_type = 0x2; + if (mode == NAND_ECC_WRITE) { + bch_wrapmode = 0x01; + eccsize0 = 0; /* extra bits in nibbles per sector */ + eccsize1 = 52; /* OOB bits in nibbles per sector */ + } else { + bch_wrapmode = 0x01; + eccsize0 = 52; /* ECC bits in nibbles per sector */ + eccsize1 = 0; /* non-ECC bits in nibbles per sector */ + } + break; default: return; } @@ -295,7 +296,7 @@ static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat, break; case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW: case OMAP_ECC_BCH8_CODE_HW: - ptr = &gpmc_cfg->bch_result_0_3[0].bch_result_x[3]; + ptr = &gpmc_cfg->bch_result_0_3[0].bch_result3; val = readl(ptr); ecc_code[i++] = (val >> 0) & 0xFF; ptr--; @@ -308,6 +309,27 @@ static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat, ptr--; } break; + case OMAP_ECC_BCH16_CODE_HW: + ptr = &gpmc_cfg->bch_result_4_6[0].bch_result6; + ecc_code[i++] = (readl(ptr) >> 8) & 0xFF; + ecc_code[i++] = (readl(ptr) >> 0) & 0xFF; + ptr--; + for (j = 0; j < 2; j++) { + ecc_code[i++] = (readl(ptr) >> 24) & 0xFF; + ecc_code[i++] = (readl(ptr) >> 16) & 0xFF; + ecc_code[i++] = (readl(ptr) >> 8) & 0xFF; + ecc_code[i++] = (readl(ptr) >> 0) & 0xFF; + ptr--; + } + ptr = &gpmc_cfg->bch_result_0_3[0].bch_result3; + for (j = 0; j < 4; j++) { + ecc_code[i++] = (readl(ptr) >> 24) & 0xFF; + ecc_code[i++] = (readl(ptr) >> 16) & 0xFF; + ecc_code[i++] = (readl(ptr) >> 8) & 0xFF; + ecc_code[i++] = (readl(ptr) >> 0) & 0xFF; + ptr--; + } + break; default: return -EINVAL; } @@ -326,6 +348,8 @@ static int omap_calculate_ecc(struct mtd_info *mtd, const uint8_t *dat, case OMAP_ECC_BCH8_CODE_HW: ecc_code[chip->ecc.bytes - 1] = 0x00; break; + case OMAP_ECC_BCH16_CODE_HW: + break; } return 0; } @@ -376,12 +400,15 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat, case OMAP_ECC_BCH8_CODE_HW: omap_reverse_list(calc_ecc, eccbytes - 1); break; + case OMAP_ECC_BCH16_CODE_HW: + omap_reverse_list(calc_ecc, eccbytes); + break; default: return -EINVAL; } /* use elm module to check for errors */ - elm_config((enum bch_level)(bch->type)); - if (elm_check_error(calc_ecc, bch->nibbles, &error_count, error_loc)) { + elm_config(bch->type); + if (elm_check_error(calc_ecc, bch->type, &error_count, error_loc)) { printf("nand: error: uncorrectable ECC errors\n"); return -EINVAL; } @@ -392,6 +419,9 @@ static int omap_correct_data_bch(struct mtd_info *mtd, uint8_t *dat, /* 14th byte in ECC is reserved to match ROM layout */ error_max = SECTOR_BYTES + (eccbytes - 1); break; + case ECC_BCH16: + error_max = SECTOR_BYTES + eccbytes; + break; default: return -EINVAL; } @@ -614,6 +644,7 @@ static int omap_select_ecc_scheme(struct nand_chip *nand, int ecc_scheme, printf("nand: error: could not init_bch()\n"); return -ENODEV; } + bch_priv.type = ECC_BCH8; /* define ecc-layout */ ecclayout->eccbytes = nand->ecc.bytes * (pagesize / SECTOR_BYTES); @@ -638,6 +669,7 @@ static int omap_select_ecc_scheme(struct nand_chip *nand, int ecc_scheme, nand->ecc.read_page = omap_read_page_bch; /* ELM is used for ECC error detection */ elm_init(); + bch_priv.type = ECC_BCH8; /* define ecc-layout */ ecclayout->eccbytes = nand->ecc.bytes * (pagesize / SECTOR_BYTES); @@ -650,6 +682,31 @@ static int omap_select_ecc_scheme(struct nand_chip *nand, int ecc_scheme, BADBLOCK_MARKER_LENGTH; bch->ecc_scheme = OMAP_ECC_BCH8_CODE_HW; break; + case OMAP_ECC_BCH16_CODE_HW: + debug("nand: using OMAP_ECC_BCH16_CODE_HW\n"); + nand->ecc.mode = NAND_ECC_HW; + nand->ecc.size = SECTOR_BYTES; + nand->ecc.bytes = 26; + nand->ecc.strength = 16; + nand->ecc.hwctl = omap_enable_hwecc; + nand->ecc.correct = omap_correct_data_bch; + nand->ecc.calculate = omap_calculate_ecc; + nand->ecc.read_page = omap_read_page_bch; + /* ELM is used for ECC error detection */ + elm_init(); + bch_priv.type = ECC_BCH16; + /* define ecc-layout */ + ecclayout->eccbytes = nand->ecc.bytes * + (pagesize / SECTOR_BYTES); + for (i = 0; i < ecclayout->eccbytes; i++) + ecclayout->eccpos[i] = i + + BADBLOCK_MARKER_LENGTH; + ecclayout->oobfree[0].offset = i + + BADBLOCK_MARKER_LENGTH; + ecclayout->oobfree[0].length = oobsize - nand->ecc.bytes - + BADBLOCK_MARKER_LENGTH; + bch->ecc_scheme = OMAP_ECC_BCH16_CODE_HW; + break; default: debug("nand: error: ecc scheme not enabled or supported\n"); return -EINVAL; diff --git a/include/mtd/mtd-abi.h b/include/mtd/mtd-abi.h index d51c1ab..f7040be 100644 --- a/include/mtd/mtd-abi.h +++ b/include/mtd/mtd-abi.h @@ -156,13 +156,14 @@ struct nand_oobfree { }; #define MTD_MAX_OOBFREE_ENTRIES 8 +#define MTD_MAX_ECCPOS_ENTRIES 256 /* * ECC layout control structure. Exported to userspace for * diagnosis and to allow creation of raw images */ struct nand_ecclayout { uint32_t eccbytes; - uint32_t eccpos[128]; + uint32_t eccpos[MTD_MAX_ECCPOS_ENTRIES]; uint32_t oobavail; struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES]; };
With increase in NAND flash densities occurence of bit-flips has increased. Thus stronger ECC schemes are required for detecting and correcting multiple simultaneous bit-flips in same NAND page. But stronger ECC schemes have large ECC syndrome which require more space in OOB/Spare. This patch add support for BCH16_ECC: (a) BCH16_ECC can correct 16 bit-flips per 512Bytes of data. (b) BCH16_ECC generates 26-bytes of ECC syndrome / 512B. Due to (b) this scheme can only be used with NAND devices which have enough OOB to satisfy following equation: OOBsize per page >= 26 * (page-size / 512) Signed-off-by: Pekon Gupta <pekon@ti.com> --- arch/arm/include/asm/arch-am33xx/cpu.h | 15 ++++- arch/arm/include/asm/arch-am33xx/omap_gpmc.h | 4 +- drivers/mtd/nand/omap_gpmc.c | 87 +++++++++++++++++++++++----- include/mtd/mtd-abi.h | 3 +- 4 files changed, 90 insertions(+), 19 deletions(-)