[PATCH v2 0/5] mtd: rawnand: zynq: Fixes and software ECC support

[PATCH v2 0/5] mtd: rawnand: zynq: Fixes and software ECC support

[Miquel Raynal (DAVE.eu)]

This driver is deeply broken in many ways. I've tried to clean it up by:
- flagging the most hideous parts (where Micron on-die ECC support is
  hardcoded)
- enabling DT support, eg. for reading the ECC configuration
- fixing software ECC support
- aligning the software ECC OOB layout to the rest of the world and
  allow a working interoperability in Linux.

Signed-off-by: Miquel Raynal (DAVE.eu) <miquel.raynal@bootlin.com>
---
Changes in v2:
- Fix the sponsor: s/DAVE/DAVE.eu/, no other change.
- Link to v1: https://lore.kernel.org/r/20260529-dave-upstream-nand-fixes-v1-0-e5bbddb10676@bootlin.com

---
Miquel Raynal (DAVE.eu) (5):
      mtd: rawnand: zynq: Align the layout structures with the driver namespace
      mtd: rawnand: zynq: Add comments about very bad practices
      mtd: rawnand: zynq: Make sure an of node is attached
      mtd: rawnand: zynq: Do not fail the probe for no reason
      mtd: rawnand: zynq: Fix Software ECC engine support

 drivers/mtd/nand/raw/zynq_nand.c | 115 ++++++++++-----------------------------
 1 file changed, 28 insertions(+), 87 deletions(-)
---
base-commit: 744cf5d4e398c5f657e5c3eeb44d00fa5175dc85
change-id: 20260522-dave-upstream-nand-fixes-60a5336517a2

Best regards,
-- 
Miquel Raynal <miquel.raynal@bootlin.com>

[PATCH v2 1/5] mtd: rawnand: zynq: Align the layout structures with the driver namespace

[Miquel Raynal (DAVE.eu)]

nand_oob_16/64 are names already used in the core and it is very
misleading to reuse them here for something that is Zynq specific. It
works because the core structures are not exported, but it is bad
practice, so rename them to prefix them "zynq_" instead.

Signed-off-by: Miquel Raynal (DAVE.eu) <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/zynq_nand.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/zynq_nand.c b/drivers/mtd/nand/raw/zynq_nand.c
index ca1aff25a522..05b500d09613 100644
--- a/drivers/mtd/nand/raw/zynq_nand.c
+++ b/drivers/mtd/nand/raw/zynq_nand.c
@@ -185,7 +185,7 @@ static const struct zynq_nand_command_format zynq_nand_commands[] = {
 };
 
 /* Define default oob placement schemes for large and small page devices */
-static struct nand_ecclayout nand_oob_16 = {
+static struct nand_ecclayout zynq_oob_16 = {
 	.eccbytes = 3,
 	.eccpos = {0, 1, 2},
 	.oobfree = {
@@ -193,7 +193,7 @@ static struct nand_ecclayout nand_oob_16 = {
 	}
 };
 
-static struct nand_ecclayout nand_oob_64 = {
+static struct nand_ecclayout zynq_oob_64 = {
 	.eccbytes = 12,
 	.eccpos = {
 		   52, 53, 54, 55, 56, 57,
@@ -1258,9 +1258,9 @@ static int zynq_nand_probe(struct udevice *dev)
 		}
 
 		if (mtd->oobsize == 16)
-			nand_chip->ecc.layout = &nand_oob_16;
+			nand_chip->ecc.layout = &zynq_oob_16;
 		else if (mtd->oobsize == 64)
-			nand_chip->ecc.layout = &nand_oob_64;
+			nand_chip->ecc.layout = &zynq_oob_64;
 		else
 			printf("%s: No oob layout found\n", __func__);
 	}

-- 
2.53.0

[PATCH v2 2/5] mtd: rawnand: zynq: Add comments about very bad practices

[Miquel Raynal (DAVE.eu)]

The driver goes through very (very) wrong steps during initialization as
it completely hardcodes support for Micron chips.

Flag these sections as undesirable in case someone ever cares about
fixing this properly, or simply stumbles about these pieces of code and
wonders why this is there.

Signed-off-by: Miquel Raynal (DAVE.eu) <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/zynq_nand.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mtd/nand/raw/zynq_nand.c b/drivers/mtd/nand/raw/zynq_nand.c
index 05b500d09613..2d07b29ecabd 100644
--- a/drivers/mtd/nand/raw/zynq_nand.c
+++ b/drivers/mtd/nand/raw/zynq_nand.c
@@ -203,6 +203,10 @@ static struct nand_ecclayout zynq_oob_64 = {
 	}
 };
 
+/*
+ * FIXME: This is a very (Micron) specific layout, which should not reside at
+ * the spi/ level.
+ */
 static struct nand_ecclayout ondie_nand_oob_64 = {
 	.eccbytes = 32,
 
@@ -1155,6 +1159,10 @@ static int zynq_nand_probe(struct udevice *dev)
 	maf_id = nand_chip->read_byte(mtd);
 	dev_id = nand_chip->read_byte(mtd);
 
+	/*
+	 * FIXME: This is horribly wrong, it checks if Micron chips have on-die
+	 * ECC support enabled. This should be moved away.
+	 */
 	if ((maf_id == 0x2c) && ((dev_id == 0xf1) ||
 				 (dev_id == 0xa1) || (dev_id == 0xb1) ||
 				 (dev_id == 0xaa) || (dev_id == 0xba) ||

-- 
2.53.0

[PATCH v2 3/5] mtd: rawnand: zynq: Make sure an of node is attached

[Miquel Raynal (DAVE.eu)]

If no ofnode is attached by the controller driver, DT parsing does not
happen.

Signed-off-by: Miquel Raynal (DAVE.eu) <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/zynq_nand.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/nand/raw/zynq_nand.c b/drivers/mtd/nand/raw/zynq_nand.c
index 2d07b29ecabd..54c255630724 100644
--- a/drivers/mtd/nand/raw/zynq_nand.c
+++ b/drivers/mtd/nand/raw/zynq_nand.c
@@ -1103,6 +1103,11 @@ static int zynq_nand_probe(struct udevice *dev)
 		return dm_scan_fdt_dev(dev);
 	}
 
+	if (!ofnode_get_child_count(of_nand))
+		nand_set_flash_node(nand_chip, of_nand);
+	else
+		nand_set_flash_node(nand_chip, ofnode_first_subnode(of_nand));
+
 	if (ofnode_read_resource(of_nand, 0, &res)) {
 		printf("Failed to get nand resource\n");
 		return -ENODEV;

-- 
2.53.0

[PATCH v2 4/5] mtd: rawnand: zynq: Do not fail the probe for no reason

[Miquel Raynal (DAVE.eu)]

If there is no indication about 16b buswidth, no need to fail the probe,
just assume it is the (very standard) 8b case.

Signed-off-by: Miquel Raynal (DAVE.eu) <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/zynq_nand.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/zynq_nand.c b/drivers/mtd/nand/raw/zynq_nand.c
index 54c255630724..be080cbb0743 100644
--- a/drivers/mtd/nand/raw/zynq_nand.c
+++ b/drivers/mtd/nand/raw/zynq_nand.c
@@ -1134,12 +1134,6 @@ static int zynq_nand_probe(struct udevice *dev)
 	nand_chip->write_buf = zynq_nand_write_buf;
 
 	is_16bit_bw = zynq_nand_check_is_16bit_bw_flash();
-	if (is_16bit_bw == NAND_BW_UNKNOWN) {
-		printf("%s: Unable detect NAND based on MIO settings\n",
-		       __func__);
-		return -EINVAL;
-	}
-
 	if (is_16bit_bw == NAND_BW_16BIT)
 		nand_chip->options = NAND_BUSWIDTH_16;
 

-- 
2.53.0

[PATCH v2 5/5] mtd: rawnand: zynq: Fix Software ECC engine support

[Miquel Raynal (DAVE.eu)]

Software ECC engine was not partially working but more importantly was
unaligned with the default software layout that has been in place for
decades due to the fact that the driver was enforcing the hardware ECC
layout (for wrong reasons). Fix the software ECC implementation, which
has the side effect of also aligning it with Linux and making the
interoperability work.

Reported-by: Andrea Scian <andrea.scian@dave.eu>
Signed-off-by: Miquel Raynal (DAVE.eu) <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/zynq_nand.c | 88 +++++-----------------------------------
 1 file changed, 11 insertions(+), 77 deletions(-)

diff --git a/drivers/mtd/nand/raw/zynq_nand.c b/drivers/mtd/nand/raw/zynq_nand.c
index be080cbb0743..a697557c722c 100644
--- a/drivers/mtd/nand/raw/zynq_nand.c
+++ b/drivers/mtd/nand/raw/zynq_nand.c
@@ -631,34 +631,6 @@ static int zynq_nand_write_page_hwecc(struct mtd_info *mtd,
 	return 0;
 }
 
-/*
- * zynq_nand_write_page_swecc - [REPLACABLE] software ecc based page
- * write function
- * @mtd:	mtd info structure
- * @chip:	nand chip info structure
- * @buf:	data buffer
- * @oob_required: must write chip->oob_poi to OOB
- */
-static int zynq_nand_write_page_swecc(struct mtd_info *mtd,
-	struct nand_chip *chip, const u8 *buf, int oob_required, int page)
-{
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
-	u8 *ecc_calc = chip->buffers->ecccalc;
-	const u8 *p = buf;
-	u32 *eccpos = chip->ecc.layout->eccpos;
-
-	/* Software ecc calculation */
-	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
-		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
-
-	for (i = 0; i < chip->ecc.total; i++)
-		chip->oob_poi[eccpos[i]] = ecc_calc[i];
-
-	return chip->ecc.write_page_raw(mtd, chip, buf, 1, page);
-}
-
 /*
  * nand_read_page_hwecc - Hardware ECC based page read function
  * @mtd:	Pointer to the mtd info structure
@@ -736,48 +708,6 @@ static int zynq_nand_read_page_hwecc(struct mtd_info *mtd,
 	return 0;
 }
 
-/*
- * zynq_nand_read_page_swecc - [REPLACABLE] software ecc based page
- * read function
- * @mtd:	mtd info structure
- * @chip:	nand chip info structure
- * @buf:	buffer to store read data
- * @page:	page number to read
- */
-static int zynq_nand_read_page_swecc(struct mtd_info *mtd,
-	struct nand_chip *chip, u8 *buf, int oob_required,  int page)
-{
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
-	u8 *p = buf;
-	u8 *ecc_calc = chip->buffers->ecccalc;
-	u8 *ecc_code = chip->buffers->ecccode;
-	u32 *eccpos = chip->ecc.layout->eccpos;
-
-	chip->ecc.read_page_raw(mtd, chip, buf, 1, page);
-
-	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
-		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
-
-	for (i = 0; i < chip->ecc.total; i++)
-		ecc_code[i] = chip->oob_poi[eccpos[i]];
-
-	eccsteps = chip->ecc.steps;
-	p = buf;
-
-	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
-		int stat;
-
-		stat = chip->ecc.correct(mtd, p, &ecc_code[i], &ecc_calc[i]);
-		if (stat < 0)
-			mtd->ecc_stats.failed++;
-		else
-			mtd->ecc_stats.corrected += stat;
-	}
-	return 0;
-}
-
 /*
  * zynq_nand_select_chip - Select the flash device
  * @mtd:	Pointer to the mtd_info structure
@@ -1219,6 +1149,16 @@ static int zynq_nand_probe(struct udevice *dev)
 		/* Use the BBT pattern descriptors */
 		nand_chip->bbt_td = &bbt_main_descr;
 		nand_chip->bbt_md = &bbt_mirror_descr;
+	} else if (nand_chip->ecc.mode == NAND_ECC_SOFT ||
+		   nand_chip->ecc.mode == NAND_ECC_SOFT_BCH ||
+		   nand_chip->ecc.mode == NAND_ECC_NONE) {
+		/* Make sure the ECC engine is off */
+		ecc_cfg = readl(&smc->reg->emcr);
+		ecc_cfg &= ~ZYNQ_MEMC_NAND_ECC_MODE_MASK;
+		writel(ecc_cfg, &smc->reg->emcr);
+
+		nand_chip->ecc.read_page_raw = zynq_nand_read_page_raw;
+		nand_chip->ecc.write_page_raw = zynq_nand_write_page_raw;
 	} else {
 		/* Hardware ECC generates 3 bytes ECC code for each 512 bytes */
 		nand_chip->ecc.mode = NAND_ECC_HW;
@@ -1255,13 +1195,7 @@ static int zynq_nand_probe(struct udevice *dev)
 			       &smc->reg->emcr);
 			break;
 		default:
-			nand_chip->ecc.mode = NAND_ECC_SOFT;
-			nand_chip->ecc.calculate = nand_calculate_ecc;
-			nand_chip->ecc.correct = nand_correct_data;
-			nand_chip->ecc.read_page = zynq_nand_read_page_swecc;
-			nand_chip->ecc.write_page = zynq_nand_write_page_swecc;
-			nand_chip->ecc.size = 256;
-			break;
+			return -EINVAL;
 		}
 
 		if (mtd->oobsize == 16)

-- 
2.53.0

Re: [PATCH v2 0/5] mtd: rawnand: zynq: Fixes and software ECC support

[Michal Simek]

Hi Love,

On 6/2/26 12:28, Miquel Raynal (DAVE.eu) wrote:
> This driver is deeply broken in many ways. I've tried to clean it up by:
> - flagging the most hideous parts (where Micron on-die ECC support is
>    hardcoded)
> - enabling DT support, eg. for reading the ECC configuration
> - fixing software ECC support
> - aligning the software ECC OOB layout to the rest of the world and
>    allow a working interoperability in Linux.
> 
> Signed-off-by: Miquel Raynal (DAVE.eu) <miquel.raynal@bootlin.com>
> ---
> Changes in v2:
> - Fix the sponsor: s/DAVE/DAVE.eu/, no other change.
> - Link to v1: https://lore.kernel.org/r/20260529-dave-upstream-nand-fixes-v1-0-e5bbddb10676@bootlin.com
> 
> ---
> Miquel Raynal (DAVE.eu) (5):
>        mtd: rawnand: zynq: Align the layout structures with the driver namespace
>        mtd: rawnand: zynq: Add comments about very bad practices
>        mtd: rawnand: zynq: Make sure an of node is attached
>        mtd: rawnand: zynq: Do not fail the probe for no reason
>        mtd: rawnand: zynq: Fix Software ECC engine support
> 
>   drivers/mtd/nand/raw/zynq_nand.c | 115 ++++++++++-----------------------------
>   1 file changed, 28 insertions(+), 87 deletions(-)
> ---
> base-commit: 744cf5d4e398c5f657e5c3eeb44d00fa5175dc85
> change-id: 20260522-dave-upstream-nand-fixes-60a5336517a2
> 
> Best regards,

can you please test this and provide results?

Thanks,
Michal

Re: [PATCH v2 0/5] mtd: rawnand: zynq: Fixes and software ECC support

[Kumar, Love]

Hi Miquel,

On 6/3/2026 2:12 PM, Michal Simek wrote:
> Hi Love,
> 
> On 6/2/26 12:28, Miquel Raynal (DAVE.eu) wrote:
>> This driver is deeply broken in many ways. I've tried to clean it up by:
>> - flagging the most hideous parts (where Micron on-die ECC support is
>>    hardcoded)
>> - enabling DT support, eg. for reading the ECC configuration
>> - fixing software ECC support
>> - aligning the software ECC OOB layout to the rest of the world and
>>    allow a working interoperability in Linux.
>>
>> Signed-off-by: Miquel Raynal (DAVE.eu) <miquel.raynal@bootlin.com>
>> ---
>> Changes in v2:
>> - Fix the sponsor: s/DAVE/DAVE.eu/, no other change.
>> - Link to v1: https://lore.kernel.org/r/20260529-dave-upstream-nand-fixes-v1-0-e5bbddb10676@bootlin.com
>>
>> ---
>> Miquel Raynal (DAVE.eu) (5):
>>        mtd: rawnand: zynq: Align the layout structures with the driver namespace
>>        mtd: rawnand: zynq: Add comments about very bad practices
>>        mtd: rawnand: zynq: Make sure an of node is attached
>>        mtd: rawnand: zynq: Do not fail the probe for no reason
>>        mtd: rawnand: zynq: Fix Software ECC engine support
>>
>>   drivers/mtd/nand/raw/zynq_nand.c | 115 ++++++++++-----------------------------
>>   1 file changed, 28 insertions(+), 87 deletions(-)
>> ---
>> base-commit: 744cf5d4e398c5f657e5c3eeb44d00fa5175dc85
>> change-id: 20260522-dave-upstream-nand-fixes-60a5336517a2
>>
>> Best regards,
> 
> can you please test this and provide results?
> 
> Thanks,
> Michal

We are seeing issue with zynq-zc770-xm011 board with NAND write.

Below is the log snippet:

"""
U-Boot 2026.07-rc4-00024-gbc44b6a0aeb0 (Jun 10 2026 - 20:00:05 +0530)
 
CPU:   Zynq 7z020
Silicon: v3.1
Model: Xilinx ZC770 XM011 board
DRAM:  ECC disabled 1 GiB
Core:  20 devices, 15 uclasses, devicetree: board
Flash: 0 Bytes
NAND:  128 MiB
MMC:
Loading Environment from nowhere... OK
In:    serial@e0001000
Out:   serial@e0001000
Err:   serial@e0001000
Net:   No ethernet found.
U-Boot 2026.07-rc4-00024-gbc44b6a0aeb0 (Jun 10 2026 - 20:00:05 +0530)CPU:   Zynq 7z020Silicon: v3.1Model: Xilinx ZC770 XM011 boardDRAM:  ECC disabled 1 GiBCore:  20 devices, 15 uclasses, devicetree: boardFlash: 0 BytesNAND:  128 MiBMMC:   Loading Environment from nowhere... OKIn:    serial@e0001000Out:   serial@e0001000Err:   serial@e0001000Net:   No ethernet found.
Hit any key to stop autoboot: 2
Hit any key to stop autoboot: 0
Zynq> nand info
nand info
 
Device 0: nand0, sector size 128 KiB
  Page size         2048 b
  OOB size            64 b
  Erase size      131072 b
  ecc strength         0 bits
  ecc step size     2048 b
  subpagesize       2048 b
  options       0x00000000
Device 0: nand0, sector size 128 KiB  Page size         2048 b  OOB size            64 b  Erase size      131072 b  ecc strength         0 bits  ecc step size     2048 b  subpagesize       2048 b  options       0x00000000  bbt options   0x00028000
Zynq> nand erase.spread 0 0x2000000
NAND erase.spread: device 0 offset 0x0, size 0x2000000
Erasing at 0x1fa0000 --  99% complete.
Erasing at 0x1fe0000 -- 100% complete.
OK
Zynq> nand write 0x80000 0 0x2000000
 
NAND write: device 0 offset 0x0, size 0x2000000
NAND write to offset 0 failed -5
"""

Please have a look.

Thanks,
Love Kumar

Re: [PATCH v2 0/5] mtd: rawnand: zynq: Fixes and software ECC support

[Miquel Raynal]

Hi Love,

> We are seeing issue with zynq-zc770-xm011 board with NAND write.
>
> Below is the log snippet:
>
> """
> U-Boot 2026.07-rc4-00024-gbc44b6a0aeb0 (Jun 10 2026 - 20:00:05 +0530)
>  
> CPU:   Zynq 7z020
> Silicon: v3.1
> Model: Xilinx ZC770 XM011 board
> DRAM:  ECC disabled 1 GiB
> Core:  20 devices, 15 uclasses, devicetree: board
> Flash: 0 Bytes
> NAND:  128 MiB
> MMC:
> Loading Environment from nowhere... OK
> In:    serial@e0001000
> Out:   serial@e0001000
> Err:   serial@e0001000
> Net:   No ethernet found.
> U-Boot 2026.07-rc4-00024-gbc44b6a0aeb0 (Jun 10 2026 - 20:00:05
> +0530)CPU: Zynq 7z020Silicon: v3.1Model: Xilinx ZC770 XM011 boardDRAM:
> ECC disabled 1 GiBCore: 20 devices, 15 uclasses, devicetree: boardFlash:
> 0 BytesNAND: 128 MiBMMC: Loading Environment from nowhere... OKIn:
> serial@e0001000Out: serial@e0001000Err: serial@e0001000Net: No ethernet
> found.
> Hit any key to stop autoboot: 2
> Hit any key to stop autoboot: 0
> Zynq> nand info
> nand info
>  
> Device 0: nand0, sector size 128 KiB
>   Page size         2048 b
>   OOB size            64 b
>   Erase size      131072 b
>   ecc strength         0 bits
>   ecc step size     2048 b
>   subpagesize       2048 b
>   options       0x00000000
> Device 0: nand0, sector size 128 KiB Page size 2048 b OOB size 64 b
> Erase size 131072 b ecc strength 0 bits ecc step size 2048 b subpagesize
> 2048 b options 0x00000000 bbt options 0x00028000
> Zynq> nand erase.spread 0 0x2000000
> NAND erase.spread: device 0 offset 0x0, size 0x2000000
> Erasing at 0x1fa0000 --  99% complete.
> Erasing at 0x1fe0000 -- 100% complete.
> OK
> Zynq> nand write 0x80000 0 0x2000000
>  
> NAND write: device 0 offset 0x0, size 0x2000000
> NAND write to offset 0 failed -5
> """
>
> Please have a look.

We reproduced it too. I mostly focused on the read part initially but
write support matters too and my colleague Bastien in copy is working on
it.

Thanks,
Miquèl