Re: [PATCH v3 37/40] mtd: spinand: Propagate ECC information to the MTD structure

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Emil Lenngren <emil.lenngren@gmail.com>
Cc: Mason Yang <masonccyang@mxic.com.tw>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Tudor Ambarus <Tudor.Ambarus@microchip.com>,
	Julien Su <juliensu@mxic.com.tw>,
	Richard Weinberger <richard@nod.at>,
	Schrempf Frieder <frieder.schrempf@kontron.de>,
	Paul Cercueil <paul@crapouillou.net>,
	Marek Vasut <marek.vasut@gmail.com>,
	linux-mtd <linux-mtd@lists.infradead.org>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Brian Norris <computersforpeace@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 37/40] mtd: spinand: Propagate ECC information to the MTD structure
Date: Mon, 17 Feb 2020 14:31:40 +0100	[thread overview]
Message-ID: <20200217143140.6c5fa0ba@collabora.com> (raw)
In-Reply-To: <CAO1O6sdVqNFG+2O1AY1qN2Q4dUFhtAMk5Qr+Np3s_poSqFp+Og@mail.gmail.com>

On Mon, 17 Feb 2020 14:27:13 +0100
Emil Lenngren <emil.lenngren@gmail.com> wrote:

> Hi,
> 
> Den tors 19 sep. 2019 kl 21:56 skrev Miquel Raynal <miquel.raynal@bootlin.com>:
> >
> > This is done by default in the raw NAND core (nand_base.c) but was
> > missing in the SPI-NAND core. Without these two lines the ecc_strength
> > and ecc_step_size values are not exported to the user through sysfs.
> >
> > This fix depends on recent changes and should not be backported as-is.
> >
> > Fixes: 7529df465248 ("mtd: nand: Add core infrastructure to support SPI NANDs")
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/spi/core.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> > index a521eeb0d351..36b99f68da81 100644
> > --- a/drivers/mtd/nand/spi/core.c
> > +++ b/drivers/mtd/nand/spi/core.c
> > @@ -1135,6 +1135,10 @@ static int spinand_init(struct spinand_device *spinand)
> >
> >         mtd->oobavail = ret;
> >
> > +       /* Propagate ECC information to mtd_info */
> > +       mtd->ecc_strength = nand->ecc.ctx.conf.strength;
> > +       mtd->ecc_step_size = nand->ecc.ctx.conf.step_size;
> > +
> >         return 0;
> >
> >  err_cleanup_ecc_engine:
> > --  
> 
> If I follow the code flow correctly, it seems this is more important
> than just exposing the value to the user through sysfs. The
> mtd_read_oob function uses the ecc_strength and bitflip_threshold
> values to decide if it should return -EUCLEAN or 0 for a read with
> correctable bitflips:
> 
> if (mtd->ecc_strength == 0)
>         return 0; /* device lacks ecc */
> return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0;
> 
> The -EUCLEAN error code is then used in UBI for the wear level logic,
> so it would be nice if this could be fixed.
> Now both ecc_strength and bitflip_threshold is 0 for me with my SPI
> NAND flash, as exposed through sysfs, so I assume the wear level logic
> doesn't work right now.

Indeed. We should probably send this fix separately and add a Cc:stable
tag.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel