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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 29 Nov 2018 10:56:02 +0100
From:   Clément Péron <peron.clem@...il.com>
To:     frieder.schrempf@...tron.de
Cc:     Miquel Raynal <miquel.raynal@...tlin.com>,
        linux-kernel@...r.kernel.org,
        Boris Brezillon <boris.brezillon@...tlin.com>, richard@....at,
        David Woodhouse <dwmw2@...radead.org>,
        Brian Norris <computersforpeace@...il.com>,
        Marek Vašut <marek.vasut@...il.com>,
        linux-mtd@...ts.infradead.org
Subject: Re: [PATCH v2] mtd: spinand: Fix ECC status and OOB layout for
 Toshiba TC58CVG2S0H

Hi Frieder,

On Wed, 28 Nov 2018 at 14:18, Schrempf Frieder
<frieder.schrempf@...tron.de> 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@...il.com>
> Signed-off-by: Frieder Schrempf <frieder.schrempf@...tron.de>
Acked-by: Clément Péron <peron.clem@...il.com>
> ---
>  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