Skip to contentSkip to conversation

mtd: spinand: Fix incorrect parameters for on-die ECC

PATCH Applied Frieder Schrempf 2 participants 3 messages Aug 30, 2021 – Sep 1, 2021

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 mtd: spinand: Fix incorrect parameters for on-die ECC

From: Frieder Schrempf <[email protected]>

The new generic NAND ECC framework stores the configuration and requirements in separate places since commit 93ef92f6f422 ("mtd: nand: Use the new generic ECC object"). In 5.10.x The SPI NAND layer still uses only the requirements to track the ECC properties. This mismatch leads to values of zero being used for ECC strength and step_size in the SPI NAND layer wherever nanddev_get_ecc_conf() is used and therefore breaks the SPI NAND on-die ECC support in 5.10.x.

By using nanddev_get_ecc_requirements() instead of nanddev_get_ecc_conf() for SPI NAND, we make sure that the correct parameters for the detected chip are used. In later versions (5.11.x) this is fixed anyway with the implementation of the SPI NAND on-die ECC engine.

---
Changes in v2:
* Fix checkpatch error/warnings for commit message style
* Add Miquel's A-b tag

---
 drivers/mtd/nand/spi/core.c     | 6 +++---
 drivers/mtd/nand/spi/macronix.c | 6 +++---
 drivers/mtd/nand/spi/toshiba.c  | 6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)
drivers/mtd/nand/spi/core.c +3 -3
index 558d8a14810b..8794a1f6eacd 100644
@@ -419,7 +419,7 @@ static int spinand_check_ecc_status(struct spinand_device *spinand, u8 status)
 		 * fixed, so let's return the maximum possible value so that
 		 * wear-leveling layers move the data immediately.
 		 */
-		return nanddev_get_ecc_conf(nand)->strength;
+		return nanddev_get_ecc_requirements(nand)->strength;
 
 	case STATUS_ECC_UNCOR_ERROR:
 		return -EBADMSG;
@@ -1090,8 +1090,8 @@ static int spinand_init(struct spinand_device *spinand)
 	mtd->oobavail = ret;
 
 	/* Propagate ECC information to mtd_info */
-	mtd->ecc_strength = nanddev_get_ecc_conf(nand)->strength;
-	mtd->ecc_step_size = nanddev_get_ecc_conf(nand)->step_size;
+	mtd->ecc_strength = nanddev_get_ecc_requirements(nand)->strength;
+	mtd->ecc_step_size = nanddev_get_ecc_requirements(nand)->step_size;
 
 	return 0;
 
drivers/mtd/nand/spi/macronix.c +3 -3
index 8e801e4c3a00..cd7a9cacc3fb 100644
@@ -84,11 +84,11 @@ static int mx35lf1ge4ab_ecc_get_status(struct spinand_device *spinand,
 		 * data around if it's not necessary.
 		 */
 		if (mx35lf1ge4ab_get_eccsr(spinand, &eccsr))
-			return nanddev_get_ecc_conf(nand)->strength;
+			return nanddev_get_ecc_requirements(nand)->strength;
 
-		if (WARN_ON(eccsr > nanddev_get_ecc_conf(nand)->strength ||
+		if (WARN_ON(eccsr > nanddev_get_ecc_requirements(nand)->strength ||
 			    !eccsr))
-			return nanddev_get_ecc_conf(nand)->strength;
+			return nanddev_get_ecc_requirements(nand)->strength;
 
 		return eccsr;
 
drivers/mtd/nand/spi/toshiba.c +3 -3
index 21fde2875674..6fe7bd2a94d2 100644
@@ -90,12 +90,12 @@ static int tx58cxgxsxraix_ecc_get_status(struct spinand_device *spinand,
 		 * data around if it's not necessary.
 		 */
 		if (spi_mem_exec_op(spinand->spimem, &op))
-			return nanddev_get_ecc_conf(nand)->strength;
+			return nanddev_get_ecc_requirements(nand)->strength;
 
 		mbf >>= 4;
 
-		if (WARN_ON(mbf > nanddev_get_ecc_conf(nand)->strength || !mbf))
-			return nanddev_get_ecc_conf(nand)->strength;
+		if (WARN_ON(mbf > nanddev_get_ecc_requirements(nand)->strength || !mbf))
+			return nanddev_get_ecc_requirements(nand)->strength;
 
 		return mbf;
 
Signature · 1 line
2.32.0
#1 lore raw
Quoted 15 lines · Frieder Schrempf

From: Frieder Schrempf <[email protected]>

The new generic NAND ECC framework stores the configuration and requirements in separate places since commit 93ef92f6f422 ("mtd: nand: Use the new generic ECC object"). In 5.10.x The SPI NAND layer still uses only the requirements to track the ECC properties. This mismatch leads to values of zero being used for ECC strength and step_size in the SPI NAND layer wherever nanddev_get_ecc_conf() is used and therefore breaks the SPI NAND on-die ECC support in 5.10.x.

By using nanddev_get_ecc_requirements() instead of nanddev_get_ecc_conf() for SPI NAND, we make sure that the correct parameters for the detected chip are used. In later versions (5.11.x) this is fixed anyway with the implementation of the SPI NAND on-die ECC engine.

Quoted 8 lines

---
Changes in v2:
* Fix checkpatch error/warnings for commit message style
* Add Miquel's A-b tag

Now queued up, thanks.

greg k-h

maintainer Applied
#2 lore raw

Patch "mtd: spinand: Fix incorrect parameters for on-die ECC" has been added to the 5.10-stable tree

In reply to Frieder Schrempf

This is a note to let you know that I've just added the patch titled

mtd: spinand: Fix incorrect parameters for on-die ECC

to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
mtd-spinand-fix-incorrect-parameters-for-on-die-ecc.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From [email protected] Wed Sep 1 10:16:58 2021
From: Frieder Schrempf <[email protected]>
Date: Mon, 30 Aug 2021 15:02:10 +0200
Subject: mtd: spinand: Fix incorrect parameters for on-die ECC
To: Miquel Raynal <[email protected]>
Cc: Frieder Schrempf <[email protected]>, [email protected], voice INTER connect GmbH <[email protected]>, Alexander Lobakin <[email protected]>, Felix Fietkau <[email protected]>, Greg Kroah-Hartman <[email protected]>, [email protected], [email protected], Richard Weinberger <[email protected]>, YouChing Lin <[email protected]>
Message-ID: <[email protected]>

From: Frieder Schrempf <[email protected]>

The new generic NAND ECC framework stores the configuration and requirements in separate places since commit 93ef92f6f422 ("mtd: nand: Use the new generic ECC object"). In 5.10.x The SPI NAND layer still uses only the requirements to track the ECC properties. This mismatch leads to values of zero being used for ECC strength and step_size in the SPI NAND layer wherever nanddev_get_ecc_conf() is used and therefore breaks the SPI NAND on-die ECC support in 5.10.x.

By using nanddev_get_ecc_requirements() instead of nanddev_get_ecc_conf() for SPI NAND, we make sure that the correct parameters for the detected chip are used. In later versions (5.11.x) this is fixed anyway with the implementation of the SPI NAND on-die ECC engine.

---
 drivers/mtd/nand/spi/core.c     |    6 +++---
 drivers/mtd/nand/spi/macronix.c |    6 +++---
 drivers/mtd/nand/spi/toshiba.c  |    6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)
drivers/mtd/nand/spi/core.c +3 -3
@@ -419,7 +419,7 @@ static int spinand_check_ecc_status(stru
 		 * fixed, so let's return the maximum possible value so that
 		 * wear-leveling layers move the data immediately.
 		 */
-		return nanddev_get_ecc_conf(nand)->strength;
+		return nanddev_get_ecc_requirements(nand)->strength;
 
 	case STATUS_ECC_UNCOR_ERROR:
 		return -EBADMSG;
@@ -1090,8 +1090,8 @@ static int spinand_init(struct spinand_d
 	mtd->oobavail = ret;
 
 	/* Propagate ECC information to mtd_info */
-	mtd->ecc_strength = nanddev_get_ecc_conf(nand)->strength;
-	mtd->ecc_step_size = nanddev_get_ecc_conf(nand)->step_size;
+	mtd->ecc_strength = nanddev_get_ecc_requirements(nand)->strength;
+	mtd->ecc_step_size = nanddev_get_ecc_requirements(nand)->step_size;
 
 	return 0;
 
drivers/mtd/nand/spi/macronix.c +3 -3
@@ -84,11 +84,11 @@ static int mx35lf1ge4ab_ecc_get_status(s
 		 * data around if it's not necessary.
 		 */
 		if (mx35lf1ge4ab_get_eccsr(spinand, &eccsr))
-			return nanddev_get_ecc_conf(nand)->strength;
+			return nanddev_get_ecc_requirements(nand)->strength;
 
-		if (WARN_ON(eccsr > nanddev_get_ecc_conf(nand)->strength ||
+		if (WARN_ON(eccsr > nanddev_get_ecc_requirements(nand)->strength ||
 			    !eccsr))
-			return nanddev_get_ecc_conf(nand)->strength;
+			return nanddev_get_ecc_requirements(nand)->strength;
 
 		return eccsr;
 
drivers/mtd/nand/spi/toshiba.c +3 -3
@@ -90,12 +90,12 @@ static int tx58cxgxsxraix_ecc_get_status
 		 * data around if it's not necessary.
 		 */
 		if (spi_mem_exec_op(spinand->spimem, &op))
-			return nanddev_get_ecc_conf(nand)->strength;
+			return nanddev_get_ecc_requirements(nand)->strength;
 
 		mbf >>= 4;
 
-		if (WARN_ON(mbf > nanddev_get_ecc_conf(nand)->strength || !mbf))
-			return nanddev_get_ecc_conf(nand)->strength;
+		if (WARN_ON(mbf > nanddev_get_ecc_requirements(nand)->strength || !mbf))
+			return nanddev_get_ecc_requirements(nand)->strength;
 
 		return mbf;
 

Patches currently in stable-queue which might be from [email protected] are

queue-5.10/mtd-spinand-fix-incorrect-parameters-for-on-die-ecc.patch

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