On Fri, May 18, 2018 at 11:57:41AM +0200, Teresa Remmet wrote:
> The eccsteps where set wrong for OMAP_ECC_BCH8_CODE_HW_ROMCODE.
> So the ECC was only corrected for the first 512 bytes chunk of a 2k page.
>
> Moved out the ecc step iteration out of the correcting loop to make
> it more alike the generic nand functions. And made sure that
> the ECC is caclulated for all chunks.
>
> This patch is based on work of Sascha Hauer.
>
> Fixes commit dec7b4d2bf9c ("mtd: nand_omap_gpmc: fix BCH error correction").
>
> Signed-off-by: Teresa Remmet <t.remmet@xxxxxxxxx>
> ---
> Hello,
>
> this patch hopefully finally fixes the nand issue in the nand_omap_gpmc driver.
> Sascha and Daniel worked on this before but thier final solution was not correct.
> The problem was that we can not get rid of the 14 byte ECC
> size used in OMAP_ECC_BCH8_CODE_HW_ROMCODE. The generic nand write function
> needs to consider the offset, too. Or we read wrong ECC after writing.
>
> Maybe there is someone out there to test if this is also working with
> OMAP_ECC_BCH8_CODE_HW.
>
> Teresa
>
> drivers/mtd/nand/nand_omap_gpmc.c | 115 +++++++++++++++++++-------------------
> 1 file changed, 56 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_omap_gpmc.c b/drivers/mtd/nand/nand_omap_gpmc.c
> index e18ce6358a73..97bdb8f19753 100644
> --- a/drivers/mtd/nand/nand_omap_gpmc.c
> +++ b/drivers/mtd/nand/nand_omap_gpmc.c
> @@ -297,19 +297,18 @@ 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, actual_eccsize;
> const uint8_t *erased_ecc_vec;
> unsigned int err_loc[8];
> - int bitflip_count;
> int bch_max_err;
> + int bitflip_count = 0;
> + int eccflag = 0;
> + bool is_error_reported = false;
>
> - int eccsteps = (nand->ecc.mode == NAND_ECC_HW) &&
> - (nand->ecc.size == 2048) ? 4 : 1;
> int eccsize = oinfo->nand.ecc.bytes;
>
> 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;
> @@ -324,58 +323,45 @@ static int omap_correct_bch(struct mtd_info *mtd, uint8_t *dat,
> return -EINVAL;
> }
>
> - totalcount = 0;
> -
> - for (i = 0; i < eccsteps; i++) {
> - bool is_error_reported = false;
> - 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 < actual_eccsize) && (eccflag == 0); j++) {
> + if (calc_ecc[j] != 0) {
> + eccflag = 1;
> + break;
> }
The additional (eccflag == 0) check is unnecessary. You already leave
the loop one eccflag is set to one anyway. Also using bool for eccflag
looks a little better IMO.
> + }
>
> - 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, actual_eccsize) == 0) {
> + /*
> + * calc_ecc[] matches pattern for ECC
> + * (all 0xff) so this is definitely
> + * an erased-page
> + */
The function could be made a bit easier to read by returning early when
(eccflag == 0) and also when the memcmp above returns 0.
Sascha
--
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 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
