| Message ID | 20200917075213.532161-2-tudor.ambarus@microchip.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | mtd: rawnand: Fix HW ECC handling | expand |
On Thu, 2020-09-17 at 07:52:11 UTC, Tudor Ambarus wrote: > In case of valid HW ECC, where the fallback to SW ECC is not needed, > the mentioned commit breaks the "switch (ecc->placement)" statement, > but then wrongly falls through the "case NAND_ECC_SOFT" of the > parent "switch (ecc->mode)". This causes an -EINVAL in > nand_set_ecc_soft_ops(), because for the valid HW ECC cases > ecc->mode is set to NAND_ECC_HW, but the nand_set_ecc_soft_ops() > expects a NAND_ECC_SOFT mode, thus -EINVAL. > > Move the fallback to SW ECC after the setting of the ECC on host ops. > With this, when a valid HW ECC is available, we break the > "switch (ecc->mode)" statement, and when a fallback to SW ECC is > needed, we fallthrough "case NAND_ECC_SOFT". > > Fixes: d3f8ec8e979b ("mtd: rawnand: Separate the ECC engine type and the ECC byte placement") > Reported-by: Santiago Esteban <santiago.esteban@microchip.com> > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks. Miquel
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index d42832bedff5..bd30f6632fe2 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -5665,19 +5665,6 @@ static int nand_scan_tail(struct nand_chip *chip) ecc->read_oob = nand_read_oob_syndrome; if (!ecc->write_oob) ecc->write_oob = nand_write_oob_syndrome; - - if (mtd->writesize >= ecc->size) { - if (!ecc->strength) { - WARN(1, "Driver must set ecc.strength when using hardware ECC\n"); - ret = -EINVAL; - goto err_nand_manuf_cleanup; - } - break; - } - pr_warn("%d byte HW ECC not possible on %d byte page size, fallback to SW ECC\n", - ecc->size, mtd->writesize); - ecc->engine_type = NAND_ECC_ENGINE_TYPE_SOFT; - ecc->algo = NAND_ECC_ALGO_HAMMING; break; default: @@ -5686,6 +5673,19 @@ static int nand_scan_tail(struct nand_chip *chip) ret = -EINVAL; goto err_nand_manuf_cleanup; } + + if (mtd->writesize >= ecc->size) { + if (!ecc->strength) { + WARN(1, "Driver must set ecc.strength when using hardware ECC\n"); + ret = -EINVAL; + goto err_nand_manuf_cleanup; + } + break; + } + pr_warn("%d byte HW ECC not possible on %d byte page size, fallback to SW ECC\n", + ecc->size, mtd->writesize); + ecc->engine_type = NAND_ECC_ENGINE_TYPE_SOFT; + ecc->algo = NAND_ECC_ALGO_HAMMING; fallthrough; case NAND_ECC_ENGINE_TYPE_SOFT:
In case of valid HW ECC, where the fallback to SW ECC is not needed, the mentioned commit breaks the "switch (ecc->placement)" statement, but then wrongly falls through the "case NAND_ECC_SOFT" of the parent "switch (ecc->mode)". This causes an -EINVAL in nand_set_ecc_soft_ops(), because for the valid HW ECC cases ecc->mode is set to NAND_ECC_HW, but the nand_set_ecc_soft_ops() expects a NAND_ECC_SOFT mode, thus -EINVAL. Move the fallback to SW ECC after the setting of the ECC on host ops. With this, when a valid HW ECC is available, we break the "switch (ecc->mode)" statement, and when a fallback to SW ECC is needed, we fallthrough "case NAND_ECC_SOFT". Fixes: d3f8ec8e979b ("mtd: rawnand: Separate the ECC engine type and the ECC byte placement") Reported-by: Santiago Esteban <santiago.esteban@microchip.com> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- drivers/mtd/nand/raw/nand_base.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)