public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] mtd: rawnand: sunxi: Fixes user data length for H6
@ 2026-03-05 10:01 Richard Genoud
  2026-03-05 10:01 ` [PATCH v2 1/6] mtd: rawnand: sunxi: sunxi_nand_ooblayout_free code clarification Richard Genoud
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Richard Genoud @ 2026-03-05 10:01 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Wentao Liang, Maxime Ripard, Boris Brezillon, Thomas Petazzoni,
	linux-mtd, linux-arm-kernel, linux-sunxi, linux-kernel,
	Richard Genoud

This series introduces the correct usage of user data length for
H6/H616.
If the older SoCs must use a fixed value of 4 user data bytes for each
ECC step, the H6 can have a different value for each step.

In the first round, the behavior was kept as the one used by A10 when in
fact the images in the wild with the vendor kernel are using a different
layout.

The first patches are fixes/code cleaning and the last one enables using
a variable user data length for H6/H616

Changes from v1:
- gather Reviewed-by/Acked-by
- rework patch 1 subject (more a code clarification than a fix)
- fix typo in patch 2
- remove patch 4 Fixes since it's not really a fix

Richard Genoud (6):
  mtd: rawnand: sunxi: sunxi_nand_ooblayout_free code clarification
  mtd: rawnand: sunxi: fix sunxi_nfc_hw_ecc_read_extra_oob
  mtd: rawnand: sunxi: do not count BBM bytes twice
  mtd: rawnand: sunxi: replace hard coded value by a define - take2
  mtd: rawnand: sunxi: make the code mode self-explanatory
  mtd: rawnand: sunxi: introduce variable user data length

 drivers/mtd/nand/raw/sunxi_nand.c | 299 +++++++++++++++++++++++-------
 1 file changed, 227 insertions(+), 72 deletions(-)


base-commit: 11439c4635edd669ae435eec308f4ab8a0804808

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/6] mtd: rawnand: sunxi: sunxi_nand_ooblayout_free code clarification
  2026-03-05 10:01 [PATCH v2 0/6] mtd: rawnand: sunxi: Fixes user data length for H6 Richard Genoud
@ 2026-03-05 10:01 ` Richard Genoud
  2026-03-14  8:05   ` Chen-Yu Tsai
  2026-03-05 10:01 ` [PATCH v2 2/6] mtd: rawnand: sunxi: fix sunxi_nfc_hw_ecc_read_extra_oob Richard Genoud
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Richard Genoud @ 2026-03-05 10:01 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Wentao Liang, Maxime Ripard, Boris Brezillon, Thomas Petazzoni,
	linux-mtd, linux-arm-kernel, linux-sunxi, linux-kernel,
	Richard Genoud

The available length is really USER_DATA_LEN - 2 instead of just 2 (the
user data length minus the BBM length)
USER_DATA_LEN being 4, that doesn't change anything now, but if
USER_DATA_LEN changes, it will.

Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
 drivers/mtd/nand/raw/sunxi_nand.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index e66adfcca7cd..915f1240546f 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -1755,12 +1755,12 @@ static int sunxi_nand_ooblayout_free(struct mtd_info *mtd, int section,
 
 	/*
 	 * The first 2 bytes are used for BB markers, hence we
-	 * only have 2 bytes available in the first user data
+	 * only have USER_DATA_SZ - 2 bytes available in the first user data
 	 * section.
 	 */
 	if (!section && ecc->engine_type == NAND_ECC_ENGINE_TYPE_ON_HOST) {
 		oobregion->offset = 2;
-		oobregion->length = 2;
+		oobregion->length = USER_DATA_SZ - 2;
 
 		return 0;
 	}

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/6] mtd: rawnand: sunxi: fix sunxi_nfc_hw_ecc_read_extra_oob
  2026-03-05 10:01 [PATCH v2 0/6] mtd: rawnand: sunxi: Fixes user data length for H6 Richard Genoud
  2026-03-05 10:01 ` [PATCH v2 1/6] mtd: rawnand: sunxi: sunxi_nand_ooblayout_free code clarification Richard Genoud
@ 2026-03-05 10:01 ` Richard Genoud
  2026-03-05 10:01 ` [PATCH v2 3/6] mtd: rawnand: sunxi: do not count BBM bytes twice Richard Genoud
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Richard Genoud @ 2026-03-05 10:01 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Wentao Liang, Maxime Ripard, Boris Brezillon, Thomas Petazzoni,
	linux-mtd, linux-arm-kernel, linux-sunxi, linux-kernel,
	Richard Genoud

When dumping the OOB, the bytes at the end where actually copied from
the beginning of the OOB instead of current_offset.

That leads to something like:
OOB: ff ff ff ff ff ff ff ff ea 19 00 3a 83 db aa 8d
OOB: 99 09 c8 9a 90 36 35 7d aa 15 13 07 3d 97 b2 a4
OOB: a8 bb 19 b3 07 e9 f6 25 52 d7 1a 23 e2 7e 0a e4
OOB: 52 8a 09 d2 1a 86 3d cf b4 99 43 13 d3 90 33 0b
OOB: ff ff ff ff ff ff ff ff ea 19 00 3a 83 db aa 8d
OOB: 99 09 c8 9a 90 36 35 7d aa 15 13 07 3d 97 b2 a4
OOB: a8 bb 19 b3 07 e9 f6 25 52 d7 1a 23 e2 7e 0a e4
OOB: 52 8a 09 d2 1a 86 3d cf b4 99 43 13 d3 90 33 0b
instead of:
OOB: ff ff ff ff ff ff ff ff ea 19 00 3a 83 db aa 8d
OOB: 99 09 c8 9a 90 36 35 7d aa 15 13 07 3d 97 b2 a4
OOB: a8 bb 19 b3 07 e9 f6 25 52 d7 1a 23 e2 7e 0a e4
OOB: 52 8a 09 d2 1a 86 3d cf b4 99 43 13 d3 90 33 0b
OOB: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
OOB: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
OOB: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
OOB: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
(example with BCH16, user data [8,0], no scrambling)

*cur_off (offset from the beginning of the page) was compared to offset
(offset from the beginning of the OOB), and then, the
nand_change_read_column_op() sets the current position to the beginning
of the OOB instead of OOB+offset

Fixes: 15d6f118285f ("mtd: rawnand: sunxi: Stop supporting ECC_HW_SYNDROME mode")
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
 drivers/mtd/nand/raw/sunxi_nand.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 915f1240546f..8af449e548d4 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -1048,9 +1048,9 @@ static void sunxi_nfc_hw_ecc_read_extra_oob(struct nand_chip *nand,
 	if (len <= 0)
 		return;
 
-	if (!cur_off || *cur_off != offset)
-		nand_change_read_column_op(nand, mtd->writesize, NULL, 0,
-					   false);
+	if (!cur_off || *cur_off != (offset + mtd->writesize))
+		nand_change_read_column_op(nand, mtd->writesize + offset,
+					   NULL, 0, false);
 
 	if (!randomize)
 		sunxi_nfc_read_buf(nand, oob + offset, len);

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/6] mtd: rawnand: sunxi: do not count BBM bytes twice
  2026-03-05 10:01 [PATCH v2 0/6] mtd: rawnand: sunxi: Fixes user data length for H6 Richard Genoud
  2026-03-05 10:01 ` [PATCH v2 1/6] mtd: rawnand: sunxi: sunxi_nand_ooblayout_free code clarification Richard Genoud
  2026-03-05 10:01 ` [PATCH v2 2/6] mtd: rawnand: sunxi: fix sunxi_nfc_hw_ecc_read_extra_oob Richard Genoud
@ 2026-03-05 10:01 ` Richard Genoud
  2026-03-09 15:27   ` Miquel Raynal
  2026-03-05 10:01 ` [PATCH v2 4/6] mtd: rawnand: sunxi: replace hard coded value by a define - take2 Richard Genoud
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Richard Genoud @ 2026-03-05 10:01 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Wentao Liang, Maxime Ripard, Boris Brezillon, Thomas Petazzoni,
	linux-mtd, linux-arm-kernel, linux-sunxi, linux-kernel,
	Richard Genoud

BBM is part of USER_DATA section, so we should remove it twice

This was working ok because we are on the safe size, advertising that
there was 2 bytes less available than in reality.

But we can't change old platforms, since it may lead to a different ECC
strength, so, introduce a legacy flag for old platforms, and switch the
new platforms to the correct count.

Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
 drivers/mtd/nand/raw/sunxi_nand.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 8af449e548d4..d126dc18ef27 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -275,6 +275,8 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand)
  * @has_ecc_block_512:	If the ECC can handle 512B or only 1024B chuncks
  * @has_ecc_clk:	If the controller needs an ECC clock.
  * @has_mbus_clk:	If the controller needs a mbus clock.
+ * @legacy_max_strength:If the maximize strength function was off by 2 bytes
+ *			NB: this should not be used in new controllers
  * @reg_io_data:	I/O data register
  * @reg_ecc_err_cnt:	ECC error counter register
  * @reg_user_data:	User data register
@@ -304,6 +306,7 @@ struct sunxi_nfc_caps {
 	bool has_ecc_block_512;
 	bool has_ecc_clk;
 	bool has_mbus_clk;
+	bool legacy_max_strength;
 	unsigned int reg_io_data;
 	unsigned int reg_ecc_err_cnt;
 	unsigned int reg_user_data;
@@ -1805,10 +1808,22 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
 		ecc->size = 1024;
 		nsectors = mtd->writesize / ecc->size;
 
-		/* Reserve 2 bytes for the BBM */
-		bytes = (mtd->oobsize - 2) / nsectors;
+		/*
+		 * The 2 BBM bytes should not be removed from the grand total,
+		 * because they are part of the USER_DATA_SZ.
+		 * But we can't modify that for older platform since it may
+		 * result in a stronger ECC at the end, and break the
+		 * compatibility.
+		 */
+		if (nfc->caps->legacy_max_strength)
+			bytes = (mtd->oobsize - 2) / nsectors;
+		else
+			bytes = mtd->oobsize / nsectors;
 
-		/* 4 non-ECC bytes are added before each ECC bytes section */
+		/*
+		 * USER_DATA_SZ non-ECC bytes are added before each ECC bytes
+		 * section, they contain the 2 BBM bytes
+		 */
 		bytes -= USER_DATA_SZ;
 
 		/* and bytes has to be even. */
@@ -2373,6 +2388,7 @@ static const u8 sunxi_user_data_len_h6[] = {
 
 static const struct sunxi_nfc_caps sunxi_nfc_a10_caps = {
 	.has_ecc_block_512 = true,
+	.legacy_max_strength = true,
 	.reg_io_data = NFC_REG_A10_IO_DATA,
 	.reg_ecc_err_cnt = NFC_REG_A10_ECC_ERR_CNT,
 	.reg_user_data = NFC_REG_A10_USER_DATA,
@@ -2394,6 +2410,7 @@ static const struct sunxi_nfc_caps sunxi_nfc_a10_caps = {
 static const struct sunxi_nfc_caps sunxi_nfc_a23_caps = {
 	.has_mdma = true,
 	.has_ecc_block_512 = true,
+	.legacy_max_strength = true,
 	.reg_io_data = NFC_REG_A23_IO_DATA,
 	.reg_ecc_err_cnt = NFC_REG_A10_ECC_ERR_CNT,
 	.reg_user_data = NFC_REG_A10_USER_DATA,

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/6] mtd: rawnand: sunxi: replace hard coded value by a define - take2
  2026-03-05 10:01 [PATCH v2 0/6] mtd: rawnand: sunxi: Fixes user data length for H6 Richard Genoud
                   ` (2 preceding siblings ...)
  2026-03-05 10:01 ` [PATCH v2 3/6] mtd: rawnand: sunxi: do not count BBM bytes twice Richard Genoud
@ 2026-03-05 10:01 ` Richard Genoud
  2026-03-05 10:01 ` [PATCH v2 5/6] mtd: rawnand: sunxi: make the code mode self-explanatory Richard Genoud
  2026-03-05 10:01 ` [PATCH v2 6/6] mtd: rawnand: sunxi: introduce variable user data length Richard Genoud
  5 siblings, 0 replies; 14+ messages in thread
From: Richard Genoud @ 2026-03-05 10:01 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Wentao Liang, Maxime Ripard, Boris Brezillon, Thomas Petazzoni,
	linux-mtd, linux-arm-kernel, linux-sunxi, linux-kernel,
	Richard Genoud

The user data length (4) has been replaced almost all over the file, but
2 places were forgotten.

The user data is placed before the ECC, for each step.
So, in sunxi_nfc_hw_ecc_read_extra_oob(), the offset of the user data in
OOB is indeed ((ecc->bytes + USER_DATA_SZ) * ecc->steps);

And in sunxi_nand_ooblayout_ecc(), the offset of the ECC chunk in OOB is
the same offset plus the current user data size:
section * (ecc->bytes + USER_DATA_SZ) + USER_DATA_SZ;

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
 drivers/mtd/nand/raw/sunxi_nand.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index d126dc18ef27..0b0b5349f446 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -1045,7 +1045,7 @@ static void sunxi_nfc_hw_ecc_read_extra_oob(struct nand_chip *nand,
 {
 	struct mtd_info *mtd = nand_to_mtd(nand);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
-	int offset = ((ecc->bytes + 4) * ecc->steps);
+	int offset = ((ecc->bytes + USER_DATA_SZ) * ecc->steps);
 	int len = mtd->oobsize - offset;
 
 	if (len <= 0)
@@ -1741,7 +1741,7 @@ static int sunxi_nand_ooblayout_ecc(struct mtd_info *mtd, int section,
 	if (section >= ecc->steps)
 		return -ERANGE;
 
-	oobregion->offset = section * (ecc->bytes + USER_DATA_SZ) + 4;
+	oobregion->offset = section * (ecc->bytes + USER_DATA_SZ) + USER_DATA_SZ;
 	oobregion->length = ecc->bytes;
 
 	return 0;

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/6] mtd: rawnand: sunxi: make the code mode self-explanatory
  2026-03-05 10:01 [PATCH v2 0/6] mtd: rawnand: sunxi: Fixes user data length for H6 Richard Genoud
                   ` (3 preceding siblings ...)
  2026-03-05 10:01 ` [PATCH v2 4/6] mtd: rawnand: sunxi: replace hard coded value by a define - take2 Richard Genoud
@ 2026-03-05 10:01 ` Richard Genoud
  2026-03-09 16:50   ` Miquel Raynal
  2026-03-05 10:01 ` [PATCH v2 6/6] mtd: rawnand: sunxi: introduce variable user data length Richard Genoud
  5 siblings, 1 reply; 14+ messages in thread
From: Richard Genoud @ 2026-03-05 10:01 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Wentao Liang, Maxime Ripard, Boris Brezillon, Thomas Petazzoni,
	linux-mtd, linux-arm-kernel, linux-sunxi, linux-kernel,
	Richard Genoud

In sunxi_nfc_hw_ecc_{read,write}_chunk(), the ECC step was force to 0,
the reason is not trivial to get when reading the code.

The explanation is that, from the NAND flash controller perspective, we
are indeed at step 0 for user data length and ECC errors.

Just add a const value with an explanation to clarify things.

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
 drivers/mtd/nand/raw/sunxi_nand.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 0b0b5349f446..ca701c75cec5 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -963,6 +963,8 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct nand_chip *nand,
 	u32 pattern_found;
 	bool erased;
 	int ret;
+	/* From the controller point of view, we are at step 0 */
+	const int nfc_step = 0;
 
 	if (*cur_off != data_off)
 		nand_change_read_column_op(nand, data_off, NULL, 0, false);
@@ -977,7 +979,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct nand_chip *nand,
 		return ret;
 
 	sunxi_nfc_reset_user_data_len(nfc);
-	sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
+	sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
 	sunxi_nfc_randomizer_config(nand, page, false);
 	sunxi_nfc_randomizer_enable(nand);
 	writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP,
@@ -993,10 +995,9 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct nand_chip *nand,
 	pattern_found = readl(nfc->regs + nfc->caps->reg_pat_found);
 	pattern_found = field_get(NFC_ECC_PAT_FOUND_MSK(nfc), pattern_found);
 
-	ret = sunxi_nfc_hw_ecc_correct(nand, data, oob_required ? oob : NULL, 0,
-				       readl(nfc->regs + NFC_REG_ECC_ST),
-				       pattern_found,
-				       &erased);
+	ret = sunxi_nfc_hw_ecc_correct(nand, data, oob_required ? oob : NULL,
+				       nfc_step, readl(nfc->regs + NFC_REG_ECC_ST),
+				       pattern_found, &erased);
 	if (erased)
 		return 1;
 
@@ -1029,7 +1030,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct nand_chip *nand,
 			sunxi_nfc_randomizer_read_buf(nand, oob, ecc->bytes + USER_DATA_SZ,
 						      true, page);
 
-			sunxi_nfc_hw_ecc_get_prot_oob_bytes(nand, oob, 0,
+			sunxi_nfc_hw_ecc_get_prot_oob_bytes(nand, oob, nfc_step,
 							    bbm, page);
 		}
 	}
@@ -1207,6 +1208,8 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct nand_chip *nand,
 	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
 	int ret;
+	/* From the controller point of view, we are at step 0 */
+	const int nfc_step = 0;
 
 	if (data_off != *cur_off)
 		nand_change_write_column_op(nand, data_off, NULL, 0, false);
@@ -1223,8 +1226,8 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct nand_chip *nand,
 	sunxi_nfc_randomizer_config(nand, page, false);
 	sunxi_nfc_randomizer_enable(nand);
 	sunxi_nfc_reset_user_data_len(nfc);
-	sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
-	sunxi_nfc_hw_ecc_set_prot_oob_bytes(nand, oob, 0, bbm, page);
+	sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
+	sunxi_nfc_hw_ecc_set_prot_oob_bytes(nand, oob, nfc_step, bbm, page);
 
 	writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD |
 	       NFC_ACCESS_DIR | NFC_ECC_OP,

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 6/6] mtd: rawnand: sunxi: introduce variable user data length
  2026-03-05 10:01 [PATCH v2 0/6] mtd: rawnand: sunxi: Fixes user data length for H6 Richard Genoud
                   ` (4 preceding siblings ...)
  2026-03-05 10:01 ` [PATCH v2 5/6] mtd: rawnand: sunxi: make the code mode self-explanatory Richard Genoud
@ 2026-03-05 10:01 ` Richard Genoud
  2026-03-12 14:41   ` Miquel Raynal
  5 siblings, 1 reply; 14+ messages in thread
From: Richard Genoud @ 2026-03-05 10:01 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Chen-Yu Tsai, Jernej Skrabec, Samuel Holland
  Cc: Wentao Liang, Maxime Ripard, Boris Brezillon, Thomas Petazzoni,
	linux-mtd, linux-arm-kernel, linux-sunxi, linux-kernel,
	Richard Genoud

In Allwinner SoCs, user data can be added in OOB before each ECC data.
For older SoCs like A10, the user data size was the size of a register
(4 bytes) and was mandatory before each ECC step.
So, the A10 OOB Layout is:
[4Bytes USER_DATA_STEP0] [ECC_STEP0 bytes]
[4bytes USER_DATA_STEP1] [ECC_STEP1 bytes]
...
NB: the BBM is stored at the beginning of the USER_DATA_STEP0.

Now, for H6/H616 NAND flash controller, this user data can have a
different size for each step.
And the vendor has chosen a different layout from the one on A10, using
8 bytes for step 0 and nothing for further steps:
[8bytes USER_DATA_STEP0] [ECC_STEP0 bytes] [ECC_STEP1 bytes]...
(Still with BBM stored at the beginning of the USER_DATA_STEP0)

In order to be compatible with this layout, the current one for H6/H616
has to be changed.

Fixes: 88fd4e4deae8 ("mtd: rawnand: sunxi: Add support for H616 nand controller")
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
---
 drivers/mtd/nand/raw/sunxi_nand.c | 267 ++++++++++++++++++++++--------
 1 file changed, 201 insertions(+), 66 deletions(-)

diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index ca701c75cec5..ad4788987b17 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -207,17 +207,6 @@
 
 #define NFC_MAX_CS		7
 
-/*
- * On A10/A23, this is the size of the NDFC User Data Register, containing the
- * mandatory user data bytes following the ECC for each ECC step.
- * Thus, for each ECC step, we need the ECC bytes + USER_DATA_SZ.
- * Those bits are currently unsused, and kept as default value 0xffffffff.
- *
- * On H6/H616, this size became configurable, from 0 bytes to 32, via the
- * USER_DATA_LEN registers.
- */
-#define USER_DATA_SZ 4
-
 /**
  * struct sunxi_nand_chip_sel - stores information related to NAND Chip Select
  *
@@ -300,6 +289,7 @@ static inline struct sunxi_nand_chip *to_sunxi_nand(struct nand_chip *nand)
  *			bytes to write
  * @nuser_data_tab:	Size of @user_data_len_tab
  * @sram_size:		Size of the NAND controller SRAM
+ * @user_data_len	Function returning the user data length for a step
  */
 struct sunxi_nfc_caps {
 	bool has_mdma;
@@ -326,6 +316,7 @@ struct sunxi_nfc_caps {
 	unsigned int nuser_data_tab;
 	unsigned int max_ecc_steps;
 	int sram_size;
+	unsigned int (*user_data_len)(int step);
 };
 
 /**
@@ -824,11 +815,40 @@ static inline u32 sunxi_nfc_buf_to_user_data(const u8 *buf)
 }
 
 static void sunxi_nfc_hw_ecc_get_prot_oob_bytes(struct nand_chip *nand, u8 *oob,
-						int step, bool bbm, int page)
+						int step, bool bbm, int page,
+						unsigned int user_data_sz)
 {
 	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+	u32 user_data;
 
-	sunxi_nfc_user_data_to_buf(readl(nfc->regs + NFC_REG_USER_DATA(nfc, step)), oob);
+	if (!nfc->caps->reg_user_data_len) {
+		/*
+		 * For A10, the user data for step n is in the nth
+		 * REG_USER_DATA
+		 */
+		user_data = readl(nfc->regs + NFC_REG_USER_DATA(nfc, step));
+		sunxi_nfc_user_data_to_buf(user_data, oob);
+	} else {
+		/*
+		 * For H6 NAND controller, the user data for all steps is
+		 * contained in 32 user data registers, but not at a specific
+		 * offset for each step, they are just concatenated.
+		 */
+		unsigned int user_data_off = 0;
+		unsigned int reg_off;
+		u8 *ptr = oob;
+		unsigned int i;
+
+		for (i = 0; i < step; i++)
+			user_data_off += nfc->caps->user_data_len(i);
+
+		user_data_off /= 4;
+		for (i = 0; i < user_data_sz / 4; i++, ptr += 4) {
+			reg_off = NFC_REG_USER_DATA(nfc, user_data_off + i);
+			user_data = readl(nfc->regs + reg_off);
+			sunxi_nfc_user_data_to_buf(user_data, ptr);
+		}
+	}
 
 	/* De-randomize the Bad Block Marker. */
 	if (bbm && (nand->options & NAND_NEED_SCRAMBLING))
@@ -887,17 +907,45 @@ static void sunxi_nfc_hw_ecc_set_prot_oob_bytes(struct nand_chip *nand,
 						bool bbm, int page)
 {
 	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
-	u8 user_data[USER_DATA_SZ];
+	unsigned int user_data_sz = nfc->caps->user_data_len(step);
+	u8 *user_data = NULL;
 
 	/* Randomize the Bad Block Marker. */
 	if (bbm && (nand->options & NAND_NEED_SCRAMBLING)) {
-		memcpy(user_data, oob, sizeof(user_data));
+		user_data = kmalloc(user_data_sz, GFP_KERNEL);
+		memcpy(user_data, oob, user_data_sz);
 		sunxi_nfc_randomize_bbm(nand, page, user_data);
 		oob = user_data;
 	}
 
-	writel(sunxi_nfc_buf_to_user_data(oob),
-	       nfc->regs + NFC_REG_USER_DATA(nfc, step));
+	if (!nfc->caps->reg_user_data_len) {
+		/*
+		 * For A10, the user data for step n is in the nth
+		 * REG_USER_DATA
+		 */
+		writel(sunxi_nfc_buf_to_user_data(oob),
+		       nfc->regs + NFC_REG_USER_DATA(nfc, step));
+	} else {
+		/*
+		 * For H6 NAND controller, the user data for all steps is
+		 * contained in 32 user data registers, but not at a specific
+		 * offset for each step, they are just concatenated.
+		 */
+		unsigned int user_data_off = 0;
+		const u8 *ptr = oob;
+		unsigned int i;
+
+		for (i = 0; i < step; i++)
+			user_data_off += nfc->caps->user_data_len(i);
+
+		user_data_off /= 4;
+		for (i = 0; i < user_data_sz / 4; i++, ptr += 4) {
+			writel(sunxi_nfc_buf_to_user_data(ptr),
+			       nfc->regs + NFC_REG_USER_DATA(nfc, user_data_off + i));
+		}
+	}
+
+	kfree(user_data);
 }
 
 static void sunxi_nfc_hw_ecc_update_stats(struct nand_chip *nand,
@@ -918,6 +966,7 @@ static int sunxi_nfc_hw_ecc_correct(struct nand_chip *nand, u8 *data, u8 *oob,
 				    bool *erased)
 {
 	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+	unsigned int user_data_sz = nfc->caps->user_data_len(step);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
 	u32 tmp;
 
@@ -940,7 +989,7 @@ static int sunxi_nfc_hw_ecc_correct(struct nand_chip *nand, u8 *data, u8 *oob,
 			memset(data, pattern, ecc->size);
 
 		if (oob)
-			memset(oob, pattern, ecc->bytes + USER_DATA_SZ);
+			memset(oob, pattern, ecc->bytes + user_data_sz);
 
 		return 0;
 	}
@@ -955,12 +1004,14 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct nand_chip *nand,
 				       u8 *oob, int oob_off,
 				       int *cur_off,
 				       unsigned int *max_bitflips,
-				       bool bbm, bool oob_required, int page)
+				       int step, bool oob_required, int page)
 {
 	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+	unsigned int user_data_sz = nfc->caps->user_data_len(step);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
 	int raw_mode = 0;
 	u32 pattern_found;
+	bool bbm = !step;
 	bool erased;
 	int ret;
 	/* From the controller point of view, we are at step 0 */
@@ -978,8 +1029,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct nand_chip *nand,
 	if (ret)
 		return ret;
 
-	sunxi_nfc_reset_user_data_len(nfc);
-	sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
+	sunxi_nfc_set_user_data_len(nfc, user_data_sz, nfc_step);
 	sunxi_nfc_randomizer_config(nand, page, false);
 	sunxi_nfc_randomizer_enable(nand);
 	writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD | NFC_ECC_OP,
@@ -990,7 +1040,7 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct nand_chip *nand,
 	if (ret)
 		return ret;
 
-	*cur_off = oob_off + ecc->bytes + USER_DATA_SZ;
+	*cur_off = oob_off + ecc->bytes + user_data_sz;
 
 	pattern_found = readl(nfc->regs + nfc->caps->reg_pat_found);
 	pattern_found = field_get(NFC_ECC_PAT_FOUND_MSK(nfc), pattern_found);
@@ -1014,10 +1064,10 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct nand_chip *nand,
 				      ecc->size);
 
 		nand_change_read_column_op(nand, oob_off, oob,
-					   ecc->bytes + USER_DATA_SZ, false);
+					   ecc->bytes + user_data_sz, false);
 
 		ret = nand_check_erased_ecc_chunk(data,	ecc->size, oob,
-						  ecc->bytes + USER_DATA_SZ,
+						  ecc->bytes + user_data_sz,
 						  NULL, 0, ecc->strength);
 		if (ret >= 0)
 			raw_mode = 1;
@@ -1027,11 +1077,11 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct nand_chip *nand,
 		if (oob_required) {
 			nand_change_read_column_op(nand, oob_off, NULL, 0,
 						   false);
-			sunxi_nfc_randomizer_read_buf(nand, oob, ecc->bytes + USER_DATA_SZ,
+			sunxi_nfc_randomizer_read_buf(nand, oob, ecc->bytes + user_data_sz,
 						      true, page);
 
 			sunxi_nfc_hw_ecc_get_prot_oob_bytes(nand, oob, nfc_step,
-							    bbm, page);
+							    bbm, page, user_data_sz);
 		}
 	}
 
@@ -1040,13 +1090,41 @@ static int sunxi_nfc_hw_ecc_read_chunk(struct nand_chip *nand,
 	return raw_mode;
 }
 
+/*
+ * Returns the offset of the OOB for each step.
+ * (it includes the user data before the ECC data.)
+ */
+static int sunxi_get_oob_offset(struct sunxi_nfc *nfc,
+				struct nand_ecc_ctrl *ecc, int step)
+{
+	int ecc_off = step * ecc->bytes;
+	int i;
+
+	for (i = 0; i < step; i++)
+		ecc_off += nfc->caps->user_data_len(i);
+
+	return ecc_off;
+}
+
+/*
+ * Returns the offset of the ECC for each step.
+ * So, it's the same as sunxi_get_oob_offset(),
+ * but it skips the next user data.
+ */
+static int sunxi_get_ecc_offset(struct sunxi_nfc *nfc,
+				struct nand_ecc_ctrl *ecc, int step)
+{
+	return sunxi_get_oob_offset(nfc, ecc, step) + nfc->caps->user_data_len(step);
+}
+
 static void sunxi_nfc_hw_ecc_read_extra_oob(struct nand_chip *nand,
 					    u8 *oob, int *cur_off,
 					    bool randomize, int page)
 {
+	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
 	struct mtd_info *mtd = nand_to_mtd(nand);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
-	int offset = ((ecc->bytes + USER_DATA_SZ) * ecc->steps);
+	int offset = sunxi_get_oob_offset(nfc, ecc, ecc->steps);
 	int len = mtd->oobsize - offset;
 
 	if (len <= 0)
@@ -1090,7 +1168,8 @@ static int sunxi_nfc_hw_ecc_read_chunks_dma(struct nand_chip *nand, uint8_t *buf
 
 	sunxi_nfc_hw_ecc_enable(nand);
 	sunxi_nfc_reset_user_data_len(nfc);
-	sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, 0);
+	for (i = 0; i < nchunks; i++)
+		sunxi_nfc_set_user_data_len(nfc, nfc->caps->user_data_len(i), i);
 	sunxi_nfc_randomizer_config(nand, page, false);
 	sunxi_nfc_randomizer_enable(nand);
 
@@ -1125,7 +1204,8 @@ static int sunxi_nfc_hw_ecc_read_chunks_dma(struct nand_chip *nand, uint8_t *buf
 
 	for (i = 0; i < nchunks; i++) {
 		int data_off = i * ecc->size;
-		int oob_off = i * (ecc->bytes + USER_DATA_SZ);
+		unsigned int user_data_sz = nfc->caps->user_data_len(i);
+		int oob_off = sunxi_get_oob_offset(nfc, ecc, i);
 		u8 *data = buf + data_off;
 		u8 *oob = nand->oob_poi + oob_off;
 		bool erased;
@@ -1143,10 +1223,10 @@ static int sunxi_nfc_hw_ecc_read_chunks_dma(struct nand_chip *nand, uint8_t *buf
 			/* TODO: use DMA to retrieve OOB */
 			nand_change_read_column_op(nand,
 						   mtd->writesize + oob_off,
-						   oob, ecc->bytes + USER_DATA_SZ, false);
+						   oob, ecc->bytes + user_data_sz, false);
 
-			sunxi_nfc_hw_ecc_get_prot_oob_bytes(nand, oob, i,
-							    !i, page);
+			sunxi_nfc_hw_ecc_get_prot_oob_bytes(nand, oob, i, !i,
+							    page, user_data_sz);
 		}
 
 		if (erased)
@@ -1158,7 +1238,8 @@ static int sunxi_nfc_hw_ecc_read_chunks_dma(struct nand_chip *nand, uint8_t *buf
 	if (status & NFC_ECC_ERR_MSK(nfc)) {
 		for (i = 0; i < nchunks; i++) {
 			int data_off = i * ecc->size;
-			int oob_off = i * (ecc->bytes + USER_DATA_SZ);
+			unsigned int user_data_sz = nfc->caps->user_data_len(i);
+			int oob_off = sunxi_get_oob_offset(nfc, ecc, i);
 			u8 *data = buf + data_off;
 			u8 *oob = nand->oob_poi + oob_off;
 
@@ -1178,10 +1259,10 @@ static int sunxi_nfc_hw_ecc_read_chunks_dma(struct nand_chip *nand, uint8_t *buf
 			/* TODO: use DMA to retrieve OOB */
 			nand_change_read_column_op(nand,
 						   mtd->writesize + oob_off,
-						   oob, ecc->bytes + USER_DATA_SZ, false);
+						   oob, ecc->bytes + user_data_sz, false);
 
 			ret = nand_check_erased_ecc_chunk(data,	ecc->size, oob,
-							  ecc->bytes + USER_DATA_SZ,
+							  ecc->bytes + user_data_sz,
 							  NULL, 0,
 							  ecc->strength);
 			if (ret >= 0)
@@ -1202,11 +1283,13 @@ static int sunxi_nfc_hw_ecc_read_chunks_dma(struct nand_chip *nand, uint8_t *buf
 static int sunxi_nfc_hw_ecc_write_chunk(struct nand_chip *nand,
 					const u8 *data, int data_off,
 					const u8 *oob, int oob_off,
-					int *cur_off, bool bbm,
+					int *cur_off, int step,
 					int page)
 {
 	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+	unsigned int user_data_sz = nfc->caps->user_data_len(step);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
+	bool bbm = !step;
 	int ret;
 	/* From the controller point of view, we are at step 0 */
 	const int nfc_step = 0;
@@ -1225,8 +1308,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct nand_chip *nand,
 
 	sunxi_nfc_randomizer_config(nand, page, false);
 	sunxi_nfc_randomizer_enable(nand);
-	sunxi_nfc_reset_user_data_len(nfc);
-	sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, nfc_step);
+	sunxi_nfc_set_user_data_len(nfc, user_data_sz, nfc_step);
 	sunxi_nfc_hw_ecc_set_prot_oob_bytes(nand, oob, nfc_step, bbm, page);
 
 	writel(NFC_DATA_TRANS | NFC_DATA_SWAP_METHOD |
@@ -1238,7 +1320,7 @@ static int sunxi_nfc_hw_ecc_write_chunk(struct nand_chip *nand,
 	if (ret)
 		return ret;
 
-	*cur_off = oob_off + ecc->bytes + USER_DATA_SZ;
+	*cur_off = oob_off + ecc->bytes + user_data_sz;
 
 	return 0;
 }
@@ -1249,7 +1331,8 @@ static void sunxi_nfc_hw_ecc_write_extra_oob(struct nand_chip *nand,
 {
 	struct mtd_info *mtd = nand_to_mtd(nand);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
-	int offset = ((ecc->bytes + USER_DATA_SZ) * ecc->steps);
+	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+	int offset = sunxi_get_oob_offset(nfc, ecc, ecc->steps);
 	int len = mtd->oobsize - offset;
 
 	if (len <= 0)
@@ -1268,6 +1351,7 @@ static void sunxi_nfc_hw_ecc_write_extra_oob(struct nand_chip *nand,
 static int sunxi_nfc_hw_ecc_read_page(struct nand_chip *nand, uint8_t *buf,
 				      int oob_required, int page)
 {
+	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
 	struct mtd_info *mtd = nand_to_mtd(nand);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
 	unsigned int max_bitflips = 0;
@@ -1280,16 +1364,17 @@ static int sunxi_nfc_hw_ecc_read_page(struct nand_chip *nand, uint8_t *buf,
 
 	sunxi_nfc_hw_ecc_enable(nand);
 
+	sunxi_nfc_reset_user_data_len(nfc);
 	for (i = 0; i < ecc->steps; i++) {
 		int data_off = i * ecc->size;
-		int oob_off = i * (ecc->bytes + USER_DATA_SZ);
+		int oob_off = sunxi_get_oob_offset(nfc, ecc, i);
 		u8 *data = buf + data_off;
 		u8 *oob = nand->oob_poi + oob_off;
 
 		ret = sunxi_nfc_hw_ecc_read_chunk(nand, data, data_off, oob,
 						  oob_off + mtd->writesize,
 						  &cur_off, &max_bitflips,
-						  !i, oob_required, page);
+						  i, oob_required, page);
 		if (ret < 0)
 			return ret;
 		else if (ret)
@@ -1327,6 +1412,7 @@ static int sunxi_nfc_hw_ecc_read_subpage(struct nand_chip *nand,
 					 u32 data_offs, u32 readlen,
 					 u8 *bufpoi, int page)
 {
+	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
 	struct mtd_info *mtd = nand_to_mtd(nand);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
 	int ret, i, cur_off = 0;
@@ -1338,17 +1424,18 @@ static int sunxi_nfc_hw_ecc_read_subpage(struct nand_chip *nand,
 
 	sunxi_nfc_hw_ecc_enable(nand);
 
+	sunxi_nfc_reset_user_data_len(nfc);
 	for (i = data_offs / ecc->size;
 	     i < DIV_ROUND_UP(data_offs + readlen, ecc->size); i++) {
 		int data_off = i * ecc->size;
-		int oob_off = i * (ecc->bytes + USER_DATA_SZ);
+		int oob_off = sunxi_get_oob_offset(nfc, ecc, i);
 		u8 *data = bufpoi + data_off;
 		u8 *oob = nand->oob_poi + oob_off;
 
 		ret = sunxi_nfc_hw_ecc_read_chunk(nand, data, data_off,
 						  oob,
 						  oob_off + mtd->writesize,
-						  &cur_off, &max_bitflips, !i,
+						  &cur_off, &max_bitflips, i,
 						  false, page);
 		if (ret < 0)
 			return ret;
@@ -1383,6 +1470,7 @@ static int sunxi_nfc_hw_ecc_write_page(struct nand_chip *nand,
 				       const uint8_t *buf, int oob_required,
 				       int page)
 {
+	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
 	struct mtd_info *mtd = nand_to_mtd(nand);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
 	int ret, i, cur_off = 0;
@@ -1393,15 +1481,16 @@ static int sunxi_nfc_hw_ecc_write_page(struct nand_chip *nand,
 
 	sunxi_nfc_hw_ecc_enable(nand);
 
+	sunxi_nfc_reset_user_data_len(nfc);
 	for (i = 0; i < ecc->steps; i++) {
 		int data_off = i * ecc->size;
-		int oob_off = i * (ecc->bytes + USER_DATA_SZ);
+		int oob_off = sunxi_get_oob_offset(nfc, ecc, i);
 		const u8 *data = buf + data_off;
 		const u8 *oob = nand->oob_poi + oob_off;
 
 		ret = sunxi_nfc_hw_ecc_write_chunk(nand, data, data_off, oob,
 						   oob_off + mtd->writesize,
-						   &cur_off, !i, page);
+						   &cur_off, i, page);
 		if (ret)
 			return ret;
 	}
@@ -1420,6 +1509,7 @@ static int sunxi_nfc_hw_ecc_write_subpage(struct nand_chip *nand,
 					  const u8 *buf, int oob_required,
 					  int page)
 {
+	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
 	struct mtd_info *mtd = nand_to_mtd(nand);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
 	int ret, i, cur_off = 0;
@@ -1430,16 +1520,17 @@ static int sunxi_nfc_hw_ecc_write_subpage(struct nand_chip *nand,
 
 	sunxi_nfc_hw_ecc_enable(nand);
 
+	sunxi_nfc_reset_user_data_len(nfc);
 	for (i = data_offs / ecc->size;
 	     i < DIV_ROUND_UP(data_offs + data_len, ecc->size); i++) {
 		int data_off = i * ecc->size;
-		int oob_off = i * (ecc->bytes + USER_DATA_SZ);
+		int oob_off = sunxi_get_oob_offset(nfc, ecc, i);
 		const u8 *data = buf + data_off;
 		const u8 *oob = nand->oob_poi + oob_off;
 
 		ret = sunxi_nfc_hw_ecc_write_chunk(nand, data, data_off, oob,
 						   oob_off + mtd->writesize,
-						   &cur_off, !i, page);
+						   &cur_off, i, page);
 		if (ret)
 			return ret;
 	}
@@ -1473,10 +1564,12 @@ static int sunxi_nfc_hw_ecc_write_page_dma(struct nand_chip *nand,
 
 	sunxi_nfc_reset_user_data_len(nfc);
 	for (i = 0; i < ecc->steps; i++) {
-		const u8 *oob = nand->oob_poi + (i * (ecc->bytes + USER_DATA_SZ));
+		unsigned int user_data_sz = nfc->caps->user_data_len(i);
+		int oob_off = sunxi_get_oob_offset(nfc, ecc, i);
+		const u8 *oob = nand->oob_poi + oob_off;
 
 		sunxi_nfc_hw_ecc_set_prot_oob_bytes(nand, oob, i, !i, page);
-		sunxi_nfc_set_user_data_len(nfc, USER_DATA_SZ, i);
+		sunxi_nfc_set_user_data_len(nfc, user_data_sz, i);
 	}
 
 	nand_prog_page_begin_op(nand, page, 0, NULL, 0);
@@ -1740,11 +1833,12 @@ static int sunxi_nand_ooblayout_ecc(struct mtd_info *mtd, int section,
 {
 	struct nand_chip *nand = mtd_to_nand(mtd);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
+	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
 
 	if (section >= ecc->steps)
 		return -ERANGE;
 
-	oobregion->offset = section * (ecc->bytes + USER_DATA_SZ) + USER_DATA_SZ;
+	oobregion->offset = sunxi_get_ecc_offset(nfc, ecc, section);
 	oobregion->length = ecc->bytes;
 
 	return 0;
@@ -1755,18 +1849,20 @@ static int sunxi_nand_ooblayout_free(struct mtd_info *mtd, int section,
 {
 	struct nand_chip *nand = mtd_to_nand(mtd);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
+	struct sunxi_nfc *nfc = to_sunxi_nfc(nand->controller);
+	unsigned int user_data_sz = nfc->caps->user_data_len(section);
 
 	if (section > ecc->steps)
 		return -ERANGE;
 
 	/*
 	 * The first 2 bytes are used for BB markers, hence we
-	 * only have USER_DATA_SZ - 2 bytes available in the first user data
+	 * only have user_data_sz - 2 bytes available in the first user data
 	 * section.
 	 */
 	if (!section && ecc->engine_type == NAND_ECC_ENGINE_TYPE_ON_HOST) {
 		oobregion->offset = 2;
-		oobregion->length = USER_DATA_SZ - 2;
+		oobregion->length = user_data_sz - 2;
 
 		return 0;
 	}
@@ -1778,10 +1874,10 @@ static int sunxi_nand_ooblayout_free(struct mtd_info *mtd, int section,
 	if (section == ecc->steps && ecc->engine_type == NAND_ECC_ENGINE_TYPE_ON_HOST)
 		return -ERANGE;
 
-	oobregion->offset = section * (ecc->bytes + USER_DATA_SZ);
+	oobregion->offset = sunxi_get_ecc_offset(nfc, ecc, section);
 
 	if (section < ecc->steps)
-		oobregion->length = USER_DATA_SZ;
+		oobregion->length = user_data_sz;
 	else
 		oobregion->length = mtd->oobsize - oobregion->offset;
 
@@ -1802,14 +1898,18 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
 	const u8 *strengths = nfc->caps->ecc_strengths;
 	struct mtd_info *mtd = nand_to_mtd(nand);
 	struct nand_device *nanddev = mtd_to_nanddev(mtd);
+	int total_user_data_sz = 0;
 	int nsectors;
 	int i;
 
-	if (nanddev->ecc.user_conf.flags & NAND_ECC_MAXIMIZE_STRENGTH) {
-		int bytes;
+	ecc->size = 1024;
+	nsectors = mtd->writesize / ecc->size;
 
-		ecc->size = 1024;
-		nsectors = mtd->writesize / ecc->size;
+	for (i = 0; i < nsectors; i++)
+		total_user_data_sz += nfc->caps->user_data_len(i);
+
+	if (nanddev->ecc.user_conf.flags & NAND_ECC_MAXIMIZE_STRENGTH) {
+		int bytes = mtd->oobsize;
 
 		/*
 		 * The 2 BBM bytes should not be removed from the grand total,
@@ -1819,15 +1919,15 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
 		 * compatibility.
 		 */
 		if (nfc->caps->legacy_max_strength)
-			bytes = (mtd->oobsize - 2) / nsectors;
-		else
-			bytes = mtd->oobsize / nsectors;
+			bytes -= 2;
+
+		bytes -= total_user_data_sz;
 
 		/*
-		 * USER_DATA_SZ non-ECC bytes are added before each ECC bytes
-		 * section, they contain the 2 BBM bytes
+		 * Once all user data has been subtracted, the rest can be used
+		 * for ECC bytes
 		 */
-		bytes -= USER_DATA_SZ;
+		bytes /= nsectors;
 
 		/* and bytes has to be even. */
 		if (bytes % 2)
@@ -1880,7 +1980,7 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
 
 	nsectors = mtd->writesize / ecc->size;
 
-	if (mtd->oobsize < ((ecc->bytes + USER_DATA_SZ) * nsectors))
+	if (mtd->oobsize < (ecc->bytes * nsectors + total_user_data_sz))
 		return -EINVAL;
 
 	ecc->read_oob = sunxi_nfc_hw_ecc_read_oob;
@@ -2377,6 +2477,38 @@ static void sunxi_nfc_remove(struct platform_device *pdev)
 		dma_release_channel(nfc->dmac);
 }
 
+static unsigned int sunxi_user_data_len_h616(int step)
+{
+	/*
+	 * On H6/H616, the user data size became configurable,
+	 * from 0 bytes to 32, via the USER_DATA_LEN registers.
+	 *
+	 * In H616 vendor image, the user data length is 8 byte on step 0
+	 * (that includes the BBM) and 0 bytes for the rest.
+	 * So the OOB layout is:
+	 * [BBM] [BBM] [6bytes USER_DATA_STEP0] [ECC_STEP0 bytes] [ECC_STEP1 bytes]...
+	 */
+	if (step == 0)
+		return 8;
+	return 0;
+}
+
+static unsigned int sunxi_user_data_len_a10(int step)
+{
+	/*
+	 * On A10/A23, this is the size of the NDFC User Data Register,
+	 * containing the mandatory user data bytes preceding the ECC for each
+	 * ECC step (and including the BBM)
+	 * Thus, for each ECC step, we need USER_DATA_SZ + ECC bytes.
+	 *
+	 * So the layout is:
+	 * [BBM] [BBM] [2Bytes USER_DATA_STEP0] [ECC_STEP0 bytes]
+	 * [4bytes USER_DATA_STEP1] [ECC_step1 bytes]...
+	 */
+
+	return 4;
+}
+
 static const u8 sunxi_ecc_strengths_a10[] = {
 	16, 24, 28, 32, 40, 48, 56, 60, 64
 };
@@ -2408,6 +2540,7 @@ static const struct sunxi_nfc_caps sunxi_nfc_a10_caps = {
 	.nstrengths = ARRAY_SIZE(sunxi_ecc_strengths_a10),
 	.max_ecc_steps = 16,
 	.sram_size = 1024,
+	.user_data_len = &sunxi_user_data_len_a10,
 };
 
 static const struct sunxi_nfc_caps sunxi_nfc_a23_caps = {
@@ -2430,6 +2563,7 @@ static const struct sunxi_nfc_caps sunxi_nfc_a23_caps = {
 	.nstrengths = ARRAY_SIZE(sunxi_ecc_strengths_a10),
 	.max_ecc_steps = 16,
 	.sram_size = 1024,
+	.user_data_len = &sunxi_user_data_len_a10,
 };
 
 static const struct sunxi_nfc_caps sunxi_nfc_h616_caps = {
@@ -2454,6 +2588,7 @@ static const struct sunxi_nfc_caps sunxi_nfc_h616_caps = {
 	.nuser_data_tab = ARRAY_SIZE(sunxi_user_data_len_h6),
 	.max_ecc_steps = 32,
 	.sram_size = 8192,
+	.user_data_len = &sunxi_user_data_len_h616,
 };
 
 static const struct of_device_id sunxi_nfc_ids[] = {

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/6] mtd: rawnand: sunxi: do not count BBM bytes twice
  2026-03-05 10:01 ` [PATCH v2 3/6] mtd: rawnand: sunxi: do not count BBM bytes twice Richard Genoud
@ 2026-03-09 15:27   ` Miquel Raynal
  2026-03-12 10:39     ` Richard GENOUD
  0 siblings, 1 reply; 14+ messages in thread
From: Miquel Raynal @ 2026-03-09 15:27 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Richard Weinberger, Vignesh Raghavendra, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Wentao Liang, Maxime Ripard,
	Boris Brezillon, Thomas Petazzoni, linux-mtd, linux-arm-kernel,
	linux-sunxi, linux-kernel

Hello Richard,

On 05/03/2026 at 11:01:34 +01, Richard Genoud <richard.genoud@bootlin.com> wrote:

> BBM is part of USER_DATA section, so we should remove it twice

Maybe you mean "is already part" and as such the end of the sentence
should be that "we should *not* remove it twice"?

> This was working ok because we are on the safe size, advertising that
> there was 2 bytes less available than in reality.
>
> But we can't change old platforms, since it may lead to a different ECC
> strength, so, introduce a legacy flag for old platforms, and switch the
> new platforms to the correct count.
>
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>

Thanks,
Miquèl

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/6] mtd: rawnand: sunxi: make the code mode self-explanatory
  2026-03-05 10:01 ` [PATCH v2 5/6] mtd: rawnand: sunxi: make the code mode self-explanatory Richard Genoud
@ 2026-03-09 16:50   ` Miquel Raynal
  2026-03-12 10:47     ` Richard GENOUD
  0 siblings, 1 reply; 14+ messages in thread
From: Miquel Raynal @ 2026-03-09 16:50 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Richard Weinberger, Vignesh Raghavendra, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Wentao Liang, Maxime Ripard,
	Boris Brezillon, Thomas Petazzoni, linux-mtd, linux-arm-kernel,
	linux-sunxi, linux-kernel

On 05/03/2026 at 11:01:36 +01, Richard Genoud <richard.genoud@bootlin.com> wrote:

In the title: s/mode/more/?

> In sunxi_nfc_hw_ecc_{read,write}_chunk(), the ECC step was force to 0,

forced!

> the reason is not trivial to get when reading the code.
>
> The explanation is that, from the NAND flash controller perspective, we
> are indeed at step 0 for user data length and ECC errors.
>
> Just add a const value with an explanation to clarify things.
>


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/6] mtd: rawnand: sunxi: do not count BBM bytes twice
  2026-03-09 15:27   ` Miquel Raynal
@ 2026-03-12 10:39     ` Richard GENOUD
  0 siblings, 0 replies; 14+ messages in thread
From: Richard GENOUD @ 2026-03-12 10:39 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Richard Weinberger, Vignesh Raghavendra, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Wentao Liang, Maxime Ripard,
	Boris Brezillon, Thomas Petazzoni, linux-mtd, linux-arm-kernel,
	linux-sunxi, linux-kernel

Le 09/03/2026 à 16:27, Miquel Raynal a écrit :
> Hello Richard,
> 
> On 05/03/2026 at 11:01:34 +01, Richard Genoud <richard.genoud@bootlin.com> wrote:
> 
>> BBM is part of USER_DATA section, so we should remove it twice
> 
> Maybe you mean "is already part" and as such the end of the sentence
> should be that "we should *not* remove it twice"?
yes indeed.

Thanks !

> 
>> This was working ok because we are on the safe size, advertising that
>> there was 2 bytes less available than in reality.
>>
>> But we can't change old platforms, since it may lead to a different ECC
>> strength, so, introduce a legacy flag for old platforms, and switch the
>> new platforms to the correct count.
>>
>> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
> 
> Thanks,
> Miquèl



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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 5/6] mtd: rawnand: sunxi: make the code mode self-explanatory
  2026-03-09 16:50   ` Miquel Raynal
@ 2026-03-12 10:47     ` Richard GENOUD
  0 siblings, 0 replies; 14+ messages in thread
From: Richard GENOUD @ 2026-03-12 10:47 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Richard Weinberger, Vignesh Raghavendra, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Wentao Liang, Maxime Ripard,
	Boris Brezillon, Thomas Petazzoni, linux-mtd, linux-arm-kernel,
	linux-sunxi, linux-kernel

Le 09/03/2026 à 17:50, Miquel Raynal a écrit :
> On 05/03/2026 at 11:01:36 +01, Richard Genoud <richard.genoud@bootlin.com> wrote:
> 
> In the title: s/mode/more/?
> 
>> In sunxi_nfc_hw_ecc_{read,write}_chunk(), the ECC step was force to 0,
> 
> forced!

Will fix that.

Thanks!

> 
>> the reason is not trivial to get when reading the code.
>>
>> The explanation is that, from the NAND flash controller perspective, we
>> are indeed at step 0 for user data length and ECC errors.
>>
>> Just add a const value with an explanation to clarify things.
>>
> 
> 


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 6/6] mtd: rawnand: sunxi: introduce variable user data length
  2026-03-05 10:01 ` [PATCH v2 6/6] mtd: rawnand: sunxi: introduce variable user data length Richard Genoud
@ 2026-03-12 14:41   ` Miquel Raynal
  2026-03-13 13:17     ` Richard GENOUD
  0 siblings, 1 reply; 14+ messages in thread
From: Miquel Raynal @ 2026-03-12 14:41 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Richard Weinberger, Vignesh Raghavendra, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Wentao Liang, Maxime Ripard,
	Boris Brezillon, Thomas Petazzoni, linux-mtd, linux-arm-kernel,
	linux-sunxi, linux-kernel

Hello Richard,

On 05/03/2026 at 11:01:37 +01, Richard Genoud <richard.genoud@bootlin.com> wrote:

> In Allwinner SoCs, user data can be added in OOB before each ECC data.
> For older SoCs like A10, the user data size was the size of a register
> (4 bytes) and was mandatory before each ECC step.
> So, the A10 OOB Layout is:
> [4Bytes USER_DATA_STEP0] [ECC_STEP0 bytes]
> [4bytes USER_DATA_STEP1] [ECC_STEP1 bytes]
> ...
> NB: the BBM is stored at the beginning of the USER_DATA_STEP0.
>
> Now, for H6/H616 NAND flash controller, this user data can have a
> different size for each step.
> And the vendor has chosen a different layout from the one on A10, using
> 8 bytes for step 0 and nothing for further steps:
> [8bytes USER_DATA_STEP0] [ECC_STEP0 bytes] [ECC_STEP1 bytes]...
> (Still with BBM stored at the beginning of the USER_DATA_STEP0)

I would rather be in favour of not following $(random vendor) firmware
layout. Upstream, it makes probably more sense to just allow access to
the maximum number of bytes that can be covered by the ECC engine, so I
would rather be in favour of computing the maximum size that you can set
for each step, without going over the OOB size.

Once this set up, I believe adapting the driver locally (out of tree) to
match a specific vendor layout would be rather straightforward, as all
the configuration pieces would already be in place.

Thanks,
Miquèl

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 6/6] mtd: rawnand: sunxi: introduce variable user data length
  2026-03-12 14:41   ` Miquel Raynal
@ 2026-03-13 13:17     ` Richard GENOUD
  0 siblings, 0 replies; 14+ messages in thread
From: Richard GENOUD @ 2026-03-13 13:17 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Richard Weinberger, Vignesh Raghavendra, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Wentao Liang, Maxime Ripard,
	Boris Brezillon, Thomas Petazzoni, linux-mtd, linux-arm-kernel,
	linux-sunxi, linux-kernel

Le 12/03/2026 à 15:41, Miquel Raynal a écrit :
> Hello Richard,
> 
> On 05/03/2026 at 11:01:37 +01, Richard Genoud <richard.genoud@bootlin.com> wrote:
> 
>> In Allwinner SoCs, user data can be added in OOB before each ECC data.
>> For older SoCs like A10, the user data size was the size of a register
>> (4 bytes) and was mandatory before each ECC step.
>> So, the A10 OOB Layout is:
>> [4Bytes USER_DATA_STEP0] [ECC_STEP0 bytes]
>> [4bytes USER_DATA_STEP1] [ECC_STEP1 bytes]
>> ...
>> NB: the BBM is stored at the beginning of the USER_DATA_STEP0.
>>
>> Now, for H6/H616 NAND flash controller, this user data can have a
>> different size for each step.
>> And the vendor has chosen a different layout from the one on A10, using
>> 8 bytes for step 0 and nothing for further steps:
>> [8bytes USER_DATA_STEP0] [ECC_STEP0 bytes] [ECC_STEP1 bytes]...
>> (Still with BBM stored at the beginning of the USER_DATA_STEP0)
> 
> I would rather be in favour of not following $(random vendor) firmware
> layout. Upstream, it makes probably more sense to just allow access to
> the maximum number of bytes that can be covered by the ECC engine, so I
> would rather be in favour of computing the maximum size that you can set
> for each step, without going over the OOB size.
Ok, I'll write an implementation for that.

Thanks !

> 
> Once this set up, I believe adapting the driver locally (out of tree) to
> match a specific vendor layout would be rather straightforward, as all
> the configuration pieces would already be in place.
> 
> Thanks,
> Miquèl




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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/6] mtd: rawnand: sunxi: sunxi_nand_ooblayout_free code clarification
  2026-03-05 10:01 ` [PATCH v2 1/6] mtd: rawnand: sunxi: sunxi_nand_ooblayout_free code clarification Richard Genoud
@ 2026-03-14  8:05   ` Chen-Yu Tsai
  0 siblings, 0 replies; 14+ messages in thread
From: Chen-Yu Tsai @ 2026-03-14  8:05 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Jernej Skrabec, Samuel Holland, Wentao Liang, Maxime Ripard,
	Boris Brezillon, Thomas Petazzoni, linux-mtd, linux-arm-kernel,
	linux-sunxi, linux-kernel

On Thu, Mar 5, 2026 at 6:02 PM Richard Genoud
<richard.genoud@bootlin.com> wrote:
>
> The available length is really USER_DATA_LEN - 2 instead of just 2 (the
> user data length minus the BBM length)
> USER_DATA_LEN being 4, that doesn't change anything now, but if
> USER_DATA_LEN changes, it will.
>
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>

Reviewed-by: Chen-Yu Tsai <wens@kernel.org>

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-03-14  8:06 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-05 10:01 [PATCH v2 0/6] mtd: rawnand: sunxi: Fixes user data length for H6 Richard Genoud
2026-03-05 10:01 ` [PATCH v2 1/6] mtd: rawnand: sunxi: sunxi_nand_ooblayout_free code clarification Richard Genoud
2026-03-14  8:05   ` Chen-Yu Tsai
2026-03-05 10:01 ` [PATCH v2 2/6] mtd: rawnand: sunxi: fix sunxi_nfc_hw_ecc_read_extra_oob Richard Genoud
2026-03-05 10:01 ` [PATCH v2 3/6] mtd: rawnand: sunxi: do not count BBM bytes twice Richard Genoud
2026-03-09 15:27   ` Miquel Raynal
2026-03-12 10:39     ` Richard GENOUD
2026-03-05 10:01 ` [PATCH v2 4/6] mtd: rawnand: sunxi: replace hard coded value by a define - take2 Richard Genoud
2026-03-05 10:01 ` [PATCH v2 5/6] mtd: rawnand: sunxi: make the code mode self-explanatory Richard Genoud
2026-03-09 16:50   ` Miquel Raynal
2026-03-12 10:47     ` Richard GENOUD
2026-03-05 10:01 ` [PATCH v2 6/6] mtd: rawnand: sunxi: introduce variable user data length Richard Genoud
2026-03-12 14:41   ` Miquel Raynal
2026-03-13 13:17     ` Richard GENOUD

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox