Re: [PATCH] mtd: rawnand: omap: Use BCH private fields in the specific OOB layout

From: Adam Ford <aford173@gmail.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	linux-mtd@lists.infradead.org,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>
Subject: Re: [PATCH] mtd: rawnand: omap: Use BCH private fields in the specific OOB layout
Date: Tue, 19 Jan 2021 10:05:58 -0600	[thread overview]
Message-ID: <CAHCN7xKyZronVcCUXtaaC76NpYX5dLRoMXKpTB6BfYd2QeMrGw@mail.gmail.com> (raw)
In-Reply-To: <20210119155510.5655-1-miquel.raynal@bootlin.com>

On Tue, Jan 19, 2021 at 9:55 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> The OMAP driver may leverage software BCH logic to locate errors while
> using its own hardware to detect the presence of errors. This is
> achieved with a "mixed" mode which initializes manually the software
> BCH internal logic while providing its own OOB layout.
>
> The issue here comes from the fact that the BCH driver has been
> updated to only use generic NAND objects, and no longer depend on raw
> NAND structures as it is usable from SPI-NAND as well. However, at the
> end of the BCH context initialization, the driver checks the validity
> of the OOB layout. At this stage, the raw NAND fields have not been
> populated yet while being used by the layout helpers, leading to an
> invalid layout.
>
> The chosen solution here is to include the BCH structure definition
> and to refer to the BCH fields directly (de-referenced as a const
> pointer here) to know as early as possible the number of steps and ECC
> bytes which have been chosen.
>
> Note: I don't know which commit exactly triggered the error, but the
> entire migration to a generic BCH driver got merged in one go, so this
> should not be a problem for stable backports.
>
Thanks for addressing that.

Tested-by: Adam Ford <aford173@gmail.com> #logicpd-torpedo-37xx-devkit-28.dts
> Reported-by: Adam Ford <aford173@gmail.com>
> Fixes: 80fe603160a4 ("mtd: nand: ecc-bch: Stop using raw NAND structures")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/raw/omap2.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/omap2.c b/drivers/mtd/nand/raw/omap2.c
> index fbb9955f2467..2c3e65cb68f3 100644
> --- a/drivers/mtd/nand/raw/omap2.c
> +++ b/drivers/mtd/nand/raw/omap2.c
> @@ -15,6 +15,7 @@
>  #include <linux/jiffies.h>
>  #include <linux/sched.h>
>  #include <linux/mtd/mtd.h>
> +#include <linux/mtd/nand-ecc-sw-bch.h>
>  #include <linux/mtd/rawnand.h>
>  #include <linux/mtd/partitions.h>
>  #include <linux/omap-dma.h>
> @@ -1866,18 +1867,19 @@ static const struct mtd_ooblayout_ops omap_ooblayout_ops = {
>  static int omap_sw_ooblayout_ecc(struct mtd_info *mtd, int section,
>                                  struct mtd_oob_region *oobregion)
>  {
> -       struct nand_chip *chip = mtd_to_nand(mtd);
> +       struct nand_device *nand = mtd_to_nanddev(mtd);
> +       const struct nand_ecc_sw_bch_conf *engine_conf = nand->ecc.ctx.priv;
>         int off = BADBLOCK_MARKER_LENGTH;
>
> -       if (section >= chip->ecc.steps)
> +       if (section >= engine_conf->nsteps)
>                 return -ERANGE;
>
>         /*
>          * When SW correction is employed, one OMAP specific marker byte is
>          * reserved after each ECC step.
>          */
> -       oobregion->offset = off + (section * (chip->ecc.bytes + 1));
> -       oobregion->length = chip->ecc.bytes;
> +       oobregion->offset = off + (section * (engine_conf->code_size + 1));
> +       oobregion->length = engine_conf->code_size;
>
>         return 0;
>  }
> @@ -1885,7 +1887,8 @@ static int omap_sw_ooblayout_ecc(struct mtd_info *mtd, int section,
>  static int omap_sw_ooblayout_free(struct mtd_info *mtd, int section,
>                                   struct mtd_oob_region *oobregion)
>  {
> -       struct nand_chip *chip = mtd_to_nand(mtd);
> +       struct nand_device *nand = mtd_to_nanddev(mtd);
> +       const struct nand_ecc_sw_bch_conf *engine_conf = nand->ecc.ctx.priv;
>         int off = BADBLOCK_MARKER_LENGTH;
>
>         if (section)
> @@ -1895,7 +1898,7 @@ static int omap_sw_ooblayout_free(struct mtd_info *mtd, int section,
>          * When SW correction is employed, one OMAP specific marker byte is
>          * reserved after each ECC step.
>          */
> -       off += ((chip->ecc.bytes + 1) * chip->ecc.steps);
> +       off += ((engine_conf->code_size + 1) * engine_conf->nsteps);
>         if (off >= mtd->oobsize)
>                 return -ERANGE;
>
> --
> 2.20.1
>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/