On Sat, 12 Oct 2019 11:02:09 +0200
Boris Brezillon [off-list ref] wrote:

On Thu, 19 Sep 2019 21:31:03 +0200
Miquel Raynal [off-list ref] wrote:

quoted

There is currently a confusion between the ECC type/mode/provider
(eg. hardware, software, on-die or none) and the in-bad/out-of-band  

						   ^in-band

quoted

layout which is only described for hardware engines (OOB first,
syndrome).  

It's not really about in-band/out-of-band data placement (though it
also has an impact on it since free OOB bytes are sometimes protected
by ECC or placed next to it), more ECC bytes placement.

quoted

Create a new enumeration to describe this placement.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c |  5 +++++
 include/linux/mtd/rawnand.h      | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index e6c483ec191a..74e9289e931c 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c

@@ -4842,6 +4842,11 @@ static const char * const nand_ecc_modes[] = {
 	[NAND_ECC_ON_DIE]	= "on-die",
 };
 
+static const char * const nand_ecc_engine_oob_placement[] = {  

			     ^nand_ecc_placement ?

quoted

+	[NAND_ECC_SYNDROME_OOB_PLACEMENT] = "hw_syndrome",
+	[NAND_ECC_OOB_FIRST_PLACEMENT] = "hw_oob_first",  

Since this is something you introduce, I'd recommend to change the
naming here:

s/NAND_ECC_SYNDROME_OOB_PLACEMENT/NAND_ECC_PLACEMENT_INTERLEAVED/
s/hw_syndrome/ecc-interleaved/

I realize the "ecc-" prefix is unneeded, so maybe just "interleaved"
and "first".

s/NAND_ECC_OOB_FIRST_PLACEMENT/NAND_ECC_PLACEMENT_FIRST/
s/hw_oob_first/ecc-first/

quoted

+};
+
 static int of_get_nand_ecc_mode(struct device_node *np)
 {
 	const char *pm;

diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index ccdc0c314acc..89f964816f2c 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h

@@ -93,6 +93,18 @@ enum nand_ecc_mode {
 	NAND_ECC_ON_DIE,
 };
 
+/**
+ * enum nand_ecc_engine_oob_placement - NAND ECC engine OOB placement
+ * @NAND_ECC_DEFAULT_OOB_PLACEMENT: Standard layout, or not specified  

Maybe describe what the standard layout is (ECC placed at the end), and
mention that "not specified/default" means the driver can decide to put
the ECC/free-OOB bytes where he wants.

quoted

+ * @NAND_ECC_SYNDROME_OOB_PLACEMENT: Syndrome layout (interlaced)
+ * @NAND_ECC_OOB_FIRST_PLACEMENT: Free OOB bytes first
+ */
+enum nand_ecc_engine_oob_placement {
+	NAND_ECC_DEFAULT_OOB_PLACEMENT,
+	NAND_ECC_SYNDROME_OOB_PLACEMENT,
+	NAND_ECC_OOB_FIRST_PLACEMENT,
+};
+
 enum nand_ecc_algo {
 	NAND_ECC_UNKNOWN,
 	NAND_ECC_HAMMING,  

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