| Message ID | 20240507085842.108844-1-miquel.raynal@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | mtd: rawnand: Ensure ECC configuration is propagated to upper layers | expand |
Hi Miquel, On Tue, May 07, 2024 at 10:58:42AM +0200, Miquel Raynal wrote: > Until recently the "upper layer" was MTD. But following incremental > reworks to bring spi-nand support and more recently generic ECC support, > there is now an intermediate "generic NAND" layer that also needs to get > access to some values. When using "converted" ECC engines, like the > software ones, these values are already propagated correctly. But > otherwise when using good old raw NAND controller drivers, we need to > manually set these values ourselves at the end of the "scan" operation, > once these values have been negotiated. > > Without this propagation, later (generic) checks like the one warning > users that the ECC strength is not high enough might simply no longer > work. > > Fixes: 8c126720fe10 ("mtd: rawnand: Use the ECC framework nand_ecc_is_strong_enough() helper") > Cc: stable@vger.kernel.org > Reported-by: Sascha Hauer <s.hauer@pengutronix.de> > Closes: https://lore.kernel.org/all/Zhe2JtvvN1M4Ompw@pengutronix.de/ > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > > Hello Sascha, this is only compile tested, would you mind checking if > that fixes your setup? Works as expected: Tested-by: Sascha Hauer <s.hauer@pengutronix.de> Sascha > Thanks, Miquèl > > drivers/mtd/nand/raw/nand_base.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > index d7dbbd469b89..acd137dd0957 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -6301,6 +6301,7 @@ static const struct nand_ops rawnand_ops = { > static int nand_scan_tail(struct nand_chip *chip) > { > struct mtd_info *mtd = nand_to_mtd(chip); > + struct nand_device *base = &chip->base; > struct nand_ecc_ctrl *ecc = &chip->ecc; > int ret, i; > > @@ -6445,9 +6446,13 @@ static int nand_scan_tail(struct nand_chip *chip) > if (!ecc->write_oob_raw) > ecc->write_oob_raw = ecc->write_oob; > > - /* propagate ecc info to mtd_info */ > + /* Propagate ECC info to the generic NAND and MTD layers */ > mtd->ecc_strength = ecc->strength; > + if (!base->ecc.ctx.conf.strength) > + base->ecc.ctx.conf.strength = ecc->strength; > mtd->ecc_step_size = ecc->size; > + if (!base->ecc.ctx.conf.step_size) > + base->ecc.ctx.conf.step_size = ecc->size; > > /* > * Set the number of read / write steps for one page depending on ECC > @@ -6455,6 +6460,8 @@ static int nand_scan_tail(struct nand_chip *chip) > */ > if (!ecc->steps) > ecc->steps = mtd->writesize / ecc->size; > + if (!base->ecc.ctx.nsteps) > + base->ecc.ctx.nsteps = ecc->steps; > if (ecc->steps * ecc->size != mtd->writesize) { > WARN(1, "Invalid ECC parameters\n"); > ret = -EINVAL; > -- > 2.40.1 > >
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index d7dbbd469b89..acd137dd0957 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -6301,6 +6301,7 @@ static const struct nand_ops rawnand_ops = { static int nand_scan_tail(struct nand_chip *chip) { struct mtd_info *mtd = nand_to_mtd(chip); + struct nand_device *base = &chip->base; struct nand_ecc_ctrl *ecc = &chip->ecc; int ret, i; @@ -6445,9 +6446,13 @@ static int nand_scan_tail(struct nand_chip *chip) if (!ecc->write_oob_raw) ecc->write_oob_raw = ecc->write_oob; - /* propagate ecc info to mtd_info */ + /* Propagate ECC info to the generic NAND and MTD layers */ mtd->ecc_strength = ecc->strength; + if (!base->ecc.ctx.conf.strength) + base->ecc.ctx.conf.strength = ecc->strength; mtd->ecc_step_size = ecc->size; + if (!base->ecc.ctx.conf.step_size) + base->ecc.ctx.conf.step_size = ecc->size; /* * Set the number of read / write steps for one page depending on ECC @@ -6455,6 +6460,8 @@ static int nand_scan_tail(struct nand_chip *chip) */ if (!ecc->steps) ecc->steps = mtd->writesize / ecc->size; + if (!base->ecc.ctx.nsteps) + base->ecc.ctx.nsteps = ecc->steps; if (ecc->steps * ecc->size != mtd->writesize) { WARN(1, "Invalid ECC parameters\n"); ret = -EINVAL;
Until recently the "upper layer" was MTD. But following incremental reworks to bring spi-nand support and more recently generic ECC support, there is now an intermediate "generic NAND" layer that also needs to get access to some values. When using "converted" ECC engines, like the software ones, these values are already propagated correctly. But otherwise when using good old raw NAND controller drivers, we need to manually set these values ourselves at the end of the "scan" operation, once these values have been negotiated. Without this propagation, later (generic) checks like the one warning users that the ECC strength is not high enough might simply no longer work. Fixes: 8c126720fe10 ("mtd: rawnand: Use the ECC framework nand_ecc_is_strong_enough() helper") Cc: stable@vger.kernel.org Reported-by: Sascha Hauer <s.hauer@pengutronix.de> Closes: https://lore.kernel.org/all/Zhe2JtvvN1M4Ompw@pengutronix.de/ Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- Hello Sascha, this is only compile tested, would you mind checking if that fixes your setup? Thanks, Miquèl drivers/mtd/nand/raw/nand_base.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)