Re: [PATCH v2] mtd: spinand: Fix ECC status and OOB layout for Toshiba TC58CVG2S0H

[ClÃment PÃron]

Hi Frieder,

On Wed, 28 Nov 2018 at 14:18, Schrempf Frieder
<frieder.schrempf@xxxxxxxxxx> wrote:
>
> When reading the status of the on-chip ECC, the Toshiba chip returns
> two different states for reporting corrected bitflips. We should check
> for both of them.
>
> Also return the free OOB bytes as one contiguous area, instead of
> multiple sections.
>
> Suggested-by: ClÃment PÃron <peron.clem@xxxxxxxxx>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>
Acked-by: ClÃment PÃron <peron.clem@xxxxxxxxx>
> ---
>  drivers/mtd/nand/spi/toshiba.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/spi/toshiba.c b/drivers/mtd/nand/spi/toshiba.c
> index 294bcf6..9731752 100644
> --- a/drivers/mtd/nand/spi/toshiba.c
> +++ b/drivers/mtd/nand/spi/toshiba.c
> @@ -11,6 +11,7 @@
>  #include <linux/mtd/spinand.h>
>
>  #define SPINAND_MFR_TOSHIBA            0x98
> +#define TOSH_STATUS_ECC_HAS_BITFLIPS_T (3 << 4)
>
>  static SPINAND_OP_VARIANTS(read_cache_variants,
>                 SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> @@ -33,19 +34,17 @@ static int tc58cvg2s0h_ooblayout_ecc(struct mtd_info *mtd, int section,
>         region->offset = 128 + 16 * section;
>         region->length = 16;
>
> -
>         return 0;
>  }
>
>  static int tc58cvg2s0h_ooblayout_free(struct mtd_info *mtd, int section,
>                                       struct mtd_oob_region *region)
>  {
> -       if (section > 7)
> +       if (section > 0)
>                 return -ERANGE;
>
> -       region->offset = 2 + 16 * section;
> -       region->length = 14;
> -

I will just add a comment here to say that we reserve 2 bytes for BBM.

Else, it's fine for me.

Regards,
Clement

> +       region->offset = 2;
> +       region->length = 126;
>
>         return 0;
>  }
> @@ -70,6 +69,7 @@ static int tc58cvg2s0h_ecc_get_status(struct spinand_device *spinand,
>                 return -EBADMSG;
>
>         case STATUS_ECC_HAS_BITFLIPS:
> +       case TOSH_STATUS_ECC_HAS_BITFLIPS_T:
>                 /*
>                  * Let's try to retrieve the real maximum number of bitflips
>                  * in order to avoid forcing the wear-leveling layer to move
> --
> 2.7.4