[PATCH] mtd: spinand: add support for Etron SPI-NAND flashes

[PATCH] mtd: spinand: add support for Etron SPI-NAND flashes

[Weijie Gao]

This patch adds some Etron SPI-NAND flashes from 1Gb to 8Gb with
4-bits/8-bits On-die ECC support.

EM73C044VCF/EM73D044VCO/EM73E044VCE/EM73F044VCA are tested on MediaTek
filogic MT7988 reference board.

Datasheets:
https://etron.com/wp-content/uploads/2024/08/EM73C044VCF-SPI-NAND-Flash_Rev-1.03.pdf
https://etron.com/wp-content/uploads/2022/04/EM73D044VCOR-SPI-NAND-Flash_Rev-1.00.pdf
https://etron.com/wp-content/uploads/2022/04/EM78DE044VC-SPI-NAND-Flash_Rev-1.01.pdf
https://etron.com/wp-content/uploads/2024/08/EM73E044VCEG-SPI-NAND-Flash_Rev-1.00.pdf
https://etron.com/wp-content/uploads/2022/04/EM738F044VCA-SPI-NAND-Flash_Rev-1.02.pdf

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
 drivers/mtd/nand/spi/Makefile |   5 +-
 drivers/mtd/nand/spi/core.c   |   1 +
 drivers/mtd/nand/spi/etron.c  | 240 ++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h   |   1 +
 4 files changed, 245 insertions(+), 2 deletions(-)
 create mode 100644 drivers/mtd/nand/spi/etron.c

diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
index a7a0b2cb4b9..4664280c8d6 100644
--- a/drivers/mtd/nand/spi/Makefile
+++ b/drivers/mtd/nand/spi/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 spinand-objs := core.o otp.o
-spinand-objs += alliancememory.o ato.o esmt.o fmsh.o foresee.o gigadevice.o macronix.o
-spinand-objs += micron.o paragon.o skyhigh.o toshiba.o winbond.o xtx.o
+spinand-objs += alliancememory.o ato.o esmt.o etron.o fmsh.o foresee.o
+spinand-objs += gigadevice.o macronix.o micron.o paragon.o skyhigh.o toshiba.o
+spinand-objs += winbond.o xtx.o
 obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 14af4264612..b032b0fec24 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -1227,6 +1227,7 @@ static const struct spinand_manufacturer *spinand_manufacturers[] = {
 	&alliancememory_spinand_manufacturer,
 	&ato_spinand_manufacturer,
 	&esmt_c8_spinand_manufacturer,
+	&etron_spinand_manufacturer,
 	&fmsh_spinand_manufacturer,
 	&foresee_spinand_manufacturer,
 	&gigadevice_spinand_manufacturer,
diff --git a/drivers/mtd/nand/spi/etron.c b/drivers/mtd/nand/spi/etron.c
new file mode 100644
index 00000000000..35d60180b34
--- /dev/null
+++ b/drivers/mtd/nand/spi/etron.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 Etron Technology, Inc.
+ *
+ */
+#ifndef __UBOOT__
+#include <linux/device.h>
+#include <linux/kernel.h>
+#endif
+#include <linux/mtd/spinand.h>
+
+#define SPINAND_MFR_ETRON		0xD5
+
+#define STATUS_ECC_LIMIT_BITFLIPS	(3 << 4)
+
+static SPINAND_OP_VARIANTS(read_cache_variants,
+		SPINAND_PAGE_READ_FROM_CACHE_1S_4S_4S_OP(0, 1, NULL, 0, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_1S_1S_4S_OP(0, 1, NULL, 0, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_1S_2S_2S_OP(0, 1, NULL, 0, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_1S_1S_2S_OP(0, 1, NULL, 0, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_FAST_1S_1S_1S_OP(0, 1, NULL, 0, 0),
+		SPINAND_PAGE_READ_FROM_CACHE_1S_1S_1S_OP(0, 1, NULL, 0, 0));
+
+static SPINAND_OP_VARIANTS(write_cache_variants,
+		SPINAND_PROG_LOAD_1S_1S_4S_OP(true, 0, NULL, 0),
+		SPINAND_PROG_LOAD_1S_1S_1S_OP(true, 0, NULL, 0));
+
+static SPINAND_OP_VARIANTS(update_cache_variants,
+		SPINAND_PROG_LOAD_1S_1S_4S_OP(false, 0, NULL, 0),
+		SPINAND_PROG_LOAD_1S_1S_1S_OP(false, 0, NULL, 0));
+
+static int etron_ecc4_ooblayout_ecc(struct mtd_info *mtd, int section,
+				    struct mtd_oob_region *region)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+
+	if (section >= spinand->base.memorg.pagesize / mtd->ecc_step_size)
+		return -ERANGE;
+
+	region->offset = (8 * section) + 32;
+	region->length = 8;
+
+	return 0;
+}
+
+static int etron_ecc4_ooblayout_free(struct mtd_info *mtd, int section,
+				     struct mtd_oob_region *region)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+
+	if (section >= spinand->base.memorg.pagesize / mtd->ecc_step_size)
+		return -ERANGE;
+
+	if (section) {
+		region->offset = 8 * section;
+		region->length = 8;
+	} else {
+		/* section 0 has two bytes reserved for bad block mark */
+		region->offset = 2;
+		region->length = 6;
+	}
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops etron_ecc4_ooblayout = {
+	.ecc = etron_ecc4_ooblayout_ecc,
+	.rfree = etron_ecc4_ooblayout_free,
+};
+
+static int etron_ecc8_ooblayout_ecc(struct mtd_info *mtd, int section,
+				    struct mtd_oob_region *region)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+
+	if (section >= spinand->base.memorg.pagesize / mtd->ecc_step_size)
+		return -ERANGE;
+
+	region->offset = (14 * section) + 72;
+	region->length = 14;
+
+	return 0;
+}
+
+static int etron_ecc8_ooblayout_free(struct mtd_info *mtd, int section,
+				     struct mtd_oob_region *region)
+{
+	struct spinand_device *spinand = mtd_to_spinand(mtd);
+
+	if (section >= spinand->base.memorg.pagesize / mtd->ecc_step_size)
+		return -ERANGE;
+
+	if (section) {
+		region->offset = 18 * section;
+		region->length = 18;
+	} else {
+		/* section 0 has two bytes reserved for bad block mark */
+		region->offset = 2;
+		region->length = 16;
+	}
+
+	return 0;
+}
+
+static const struct mtd_ooblayout_ops etron_ecc8_ooblayout = {
+	.ecc = etron_ecc8_ooblayout_ecc,
+	.rfree = etron_ecc8_ooblayout_free,
+};
+
+static int etron_ecc_get_status(struct spinand_device *spinand, u8 status)
+{
+	struct nand_device *nand = spinand_to_nand(spinand);
+
+	switch (status & STATUS_ECC_MASK) {
+	case STATUS_ECC_NO_BITFLIPS:
+		return 0;
+
+	case STATUS_ECC_UNCOR_ERROR:
+		return -EBADMSG;
+
+	case STATUS_ECC_HAS_BITFLIPS:
+		return nand->eccreq.strength >> 1;
+
+	case STATUS_ECC_LIMIT_BITFLIPS:
+		return nand->eccreq.strength;
+
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static const struct spinand_info etron_spinand_table[] = {
+	/* EM73C 1Gb 3.3V */
+	SPINAND_INFO("EM73C044VCF",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x25),
+		     NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1),
+		     NAND_ECCREQ(4, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&etron_ecc4_ooblayout,
+				     etron_ecc_get_status)),
+	/* EM7xD 2Gb */
+	SPINAND_INFO("EM73D044VCR",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x41),
+		     NAND_MEMORG(1, 2048, 64, 64, 2048, 40, 1, 1, 1),
+		     NAND_ECCREQ(4, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&etron_ecc4_ooblayout,
+				     etron_ecc_get_status)),
+	SPINAND_INFO("EM73D044VCO",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x3A),
+		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&etron_ecc8_ooblayout,
+				     etron_ecc_get_status)),
+	SPINAND_INFO("EM78D044VCM",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x8E),
+		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&etron_ecc8_ooblayout,
+				     etron_ecc_get_status)),
+	/* EM7xE 4Gb */
+	SPINAND_INFO("EM73E044VCG",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x42),
+		     NAND_MEMORG(1, 2048, 64, 64, 4096, 80, 1, 1, 1),
+		     NAND_ECCREQ(4, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&etron_ecc4_ooblayout,
+				     etron_ecc_get_status)),
+	SPINAND_INFO("EM73E044VCE",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x3B),
+		     NAND_MEMORG(1, 2048, 128, 64, 4096, 80, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&etron_ecc8_ooblayout,
+				     etron_ecc_get_status)),
+	SPINAND_INFO("EM78E044VCD",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_ADDR, 0x8F),
+		     NAND_MEMORG(1, 2048, 128, 64, 4096, 80, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&etron_ecc8_ooblayout,
+				     etron_ecc_get_status)),
+	/* EM7xF 8Gb */
+	SPINAND_INFO("EM73F044VCA",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x2D),
+		     NAND_MEMORG(1, 4096, 256, 64, 4096, 80, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&etron_ecc8_ooblayout,
+				     etron_ecc_get_status)),
+	SPINAND_INFO("EM78F044VCA",
+		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x8D),
+		     NAND_MEMORG(1, 4096, 256, 64, 4096, 80, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     SPINAND_HAS_QE_BIT,
+		     SPINAND_ECCINFO(&etron_ecc8_ooblayout,
+				     etron_ecc_get_status)),
+};
+
+static const struct spinand_manufacturer_ops etron_spinand_manuf_ops = {
+};
+
+const struct spinand_manufacturer etron_spinand_manufacturer = {
+	.id = SPINAND_MFR_ETRON,
+	.name = "Etron",
+	.chips = etron_spinand_table,
+	.nchips = ARRAY_SIZE(etron_spinand_table),
+	.ops = &etron_spinand_manuf_ops,
+};
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index e9561c0a395..8f46c31a427 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -361,6 +361,7 @@ struct spinand_manufacturer {
 extern const struct spinand_manufacturer alliancememory_spinand_manufacturer;
 extern const struct spinand_manufacturer ato_spinand_manufacturer;
 extern const struct spinand_manufacturer esmt_c8_spinand_manufacturer;
+extern const struct spinand_manufacturer etron_spinand_manufacturer;
 extern const struct spinand_manufacturer fmsh_spinand_manufacturer;
 extern const struct spinand_manufacturer foresee_spinand_manufacturer;
 extern const struct spinand_manufacturer gigadevice_spinand_manufacturer;
-- 
2.45.2

Re: [PATCH] mtd: spinand: add support for Etron SPI-NAND flashes

[Miquel Raynal]

Hello Weijie,

Sorry for the delay, I have a couple of comments, see below.

On 29/04/2026 at 16:13:13 +08, Weijie Gao <weijie.gao@mediatek.com> wrote:

> This patch adds some Etron SPI-NAND flashes from 1Gb to 8Gb with
> 4-bits/8-bits On-die ECC support.
>
> EM73C044VCF/EM73D044VCO/EM73E044VCE/EM73F044VCA are tested on MediaTek
> filogic MT7988 reference board.
>
> Datasheets:
> https://etron.com/wp-content/uploads/2024/08/EM73C044VCF-SPI-NAND-Flash_Rev-1.03.pdf
> https://etron.com/wp-content/uploads/2022/04/EM73D044VCOR-SPI-NAND-Flash_Rev-1.00.pdf
> https://etron.com/wp-content/uploads/2022/04/EM78DE044VC-SPI-NAND-Flash_Rev-1.01.pdf
> https://etron.com/wp-content/uploads/2024/08/EM73E044VCEG-SPI-NAND-Flash_Rev-1.00.pdf
> https://etron.com/wp-content/uploads/2022/04/EM738F044VCA-SPI-NAND-Flash_Rev-1.02.pdf

I haven't checked all the chips in detail, I checked just a couple of them.

> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
> ---

[...]

> +#ifndef __UBOOT__
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#endif

Please drop this #if block

> +#include <linux/mtd/spinand.h>
> +
> +#define SPINAND_MFR_ETRON		0xD5
> +
> +#define STATUS_ECC_LIMIT_BITFLIPS	(3 << 4)

Please add a prefix aligned with Etron's namespace.

> +static SPINAND_OP_VARIANTS(read_cache_variants,
> +		SPINAND_PAGE_READ_FROM_CACHE_1S_4S_4S_OP(0, 1, NULL, 0, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_1S_1S_4S_OP(0, 1, NULL, 0, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_1S_2S_2S_OP(0, 1, NULL, 0, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_1S_1S_2S_OP(0, 1, NULL, 0, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_FAST_1S_1S_1S_OP(0, 1, NULL, 0, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_1S_1S_1S_OP(0, 1, NULL, 0, 0));
> +
> +static SPINAND_OP_VARIANTS(write_cache_variants,
> +		SPINAND_PROG_LOAD_1S_1S_4S_OP(true, 0, NULL, 0),
> +		SPINAND_PROG_LOAD_1S_1S_1S_OP(true, 0, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(update_cache_variants,
> +		SPINAND_PROG_LOAD_1S_1S_4S_OP(false, 0, NULL, 0),
> +		SPINAND_PROG_LOAD_1S_1S_1S_OP(false, 0, NULL, 0));
> +
> +static int etron_ecc4_ooblayout_ecc(struct mtd_info *mtd, int section,
> +				    struct mtd_oob_region *region)
> +{
> +	struct spinand_device *spinand = mtd_to_spinand(mtd);
> +
> +	if (section >= spinand->base.memorg.pagesize / mtd->ecc_step_size)
> +		return -ERANGE;
> +
> +	region->offset = (8 * section) + 32;
> +	region->length = 8;

This should be grouped into one big continuous area.

> +
> +	return 0;
> +}
> +
> +static int etron_ecc4_ooblayout_free(struct mtd_info *mtd, int section,
> +				     struct mtd_oob_region *region)
> +{
> +	struct spinand_device *spinand = mtd_to_spinand(mtd);
> +
> +	if (section >= spinand->base.memorg.pagesize / mtd->ecc_step_size)
> +		return -ERANGE;
> +
> +	if (section) {
> +		region->offset = 8 * section;
> +		region->length = 8;

Ditto.

And same below, in the other OOB layouts.

> +	} else {
> +		/* section 0 has two bytes reserved for bad block mark */
> +		region->offset = 2;
> +		region->length = 6;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops etron_ecc4_ooblayout = {
> +	.ecc = etron_ecc4_ooblayout_ecc,
> +	.rfree = etron_ecc4_ooblayout_free,
> +};
> +
> +static int etron_ecc8_ooblayout_ecc(struct mtd_info *mtd, int section,
> +				    struct mtd_oob_region *region)
> +{
> +	struct spinand_device *spinand = mtd_to_spinand(mtd);
> +
> +	if (section >= spinand->base.memorg.pagesize / mtd->ecc_step_size)
> +		return -ERANGE;
> +
> +	region->offset = (14 * section) + 72;
> +	region->length = 14;
> +
> +	return 0;
> +}
> +
> +static int etron_ecc8_ooblayout_free(struct mtd_info *mtd, int section,
> +				     struct mtd_oob_region *region)
> +{
> +	struct spinand_device *spinand = mtd_to_spinand(mtd);
> +
> +	if (section >= spinand->base.memorg.pagesize / mtd->ecc_step_size)
> +		return -ERANGE;
> +
> +	if (section) {
> +		region->offset = 18 * section;
> +		region->length = 18;
> +	} else {
> +		/* section 0 has two bytes reserved for bad block mark */
> +		region->offset = 2;
> +		region->length = 16;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops etron_ecc8_ooblayout = {
> +	.ecc = etron_ecc8_ooblayout_ecc,
> +	.rfree = etron_ecc8_ooblayout_free,

         ^

Should not even compile?

> +};
> +
> +static int etron_ecc_get_status(struct spinand_device *spinand, u8 status)
> +{
> +	struct nand_device *nand = spinand_to_nand(spinand);
> +
> +	switch (status & STATUS_ECC_MASK) {
> +	case STATUS_ECC_NO_BITFLIPS:
> +		return 0;
> +
> +	case STATUS_ECC_UNCOR_ERROR:
> +		return -EBADMSG;
> +
> +	case STATUS_ECC_HAS_BITFLIPS:
> +		return nand->eccreq.strength >> 1;

This is incorrect, you should probably return strength - 1 if that is
what the bit means. We already had a discussion about this in the past:
https://lore.kernel.org/linux-mtd/20210908201624.237634-1-bert@biot.com/

> +
> +	case STATUS_ECC_LIMIT_BITFLIPS:
> +		return nand->eccreq.strength;
> +
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}

Thanks,
Miquèl

Re: [PATCH] mtd: spinand: add support for Etron SPI-NAND flashes

[Miquel Raynal]

>
> [...]
>
>> +#ifndef __UBOOT__
>> +#include <linux/device.h>
>> +#include <linux/kernel.h>
>> +#endif
>
> Please drop this #if block

Actually I missed the fact that the target was U-Boot, so it is fine obviously :-)

Rest is still valid.

Re: [PATCH] mtd: spinand: add support for Etron SPI-NAND flashes

[Weijie Gao]

On Wed, 2026-05-27 at 10:26 +0200, Miquel Raynal wrote:
> Hello Weijie,
> 
> Sorry for the delay, I have a couple of comments, see below.
> 
> On 29/04/2026 at 16:13:13 +08, Weijie Gao <weijie.gao@mediatek.com>
> wrote:
> 
> > This patch adds some Etron SPI-NAND flashes from 1Gb to 8Gb with
> > 4-bits/8-bits On-die ECC support.
> > 
> > EM73C044VCF/EM73D044VCO/EM73E044VCE/EM73F044VCA are tested on
> > MediaTek
> > filogic MT7988 reference board.
> > 
> > Datasheets:
> > 
https://urldefense.com/v3/__https://etron.com/wp-content/uploads/2024/08/EM73C044VCF-SPI-NAND-Flash_Rev-1.03.pdf__;!!CTRNKA9wMg0ARbw!gspAb_ATP-dOd61dvrUFgW4djua1MqYvOWrCMpr08fy1cd0aXgBHc0DpLK1QjsvUpnqO8M17049YgIPRAD7TX0rDOTY$
> >  
> > 
https://urldefense.com/v3/__https://etron.com/wp-content/uploads/2022/04/EM73D044VCOR-SPI-NAND-Flash_Rev-1.00.pdf__;!!CTRNKA9wMg0ARbw!gspAb_ATP-dOd61dvrUFgW4djua1MqYvOWrCMpr08fy1cd0aXgBHc0DpLK1QjsvUpnqO8M17049YgIPRAD7TDU5Zmz4$
> >  
> > 
https://urldefense.com/v3/__https://etron.com/wp-content/uploads/2022/04/EM78DE044VC-SPI-NAND-Flash_Rev-1.01.pdf__;!!CTRNKA9wMg0ARbw!gspAb_ATP-dOd61dvrUFgW4djua1MqYvOWrCMpr08fy1cd0aXgBHc0DpLK1QjsvUpnqO8M17049YgIPRAD7TC0zknDQ$
> >  
> > 
https://urldefense.com/v3/__https://etron.com/wp-content/uploads/2024/08/EM73E044VCEG-SPI-NAND-Flash_Rev-1.00.pdf__;!!CTRNKA9wMg0ARbw!gspAb_ATP-dOd61dvrUFgW4djua1MqYvOWrCMpr08fy1cd0aXgBHc0DpLK1QjsvUpnqO8M17049YgIPRAD7TN5lTwzg$
> >  
> > 
https://urldefense.com/v3/__https://etron.com/wp-content/uploads/2022/04/EM738F044VCA-SPI-NAND-Flash_Rev-1.02.pdf__;!!CTRNKA9wMg0ARbw!gspAb_ATP-dOd61dvrUFgW4djua1MqYvOWrCMpr08fy1cd0aXgBHc0DpLK1QjsvUpnqO8M17049YgIPRAD7T0csUZhs$
> >  
> 
> I haven't checked all the chips in detail, I checked just a couple of
> them.
> 
> > Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
> > ---
> 
> [...]
> 
> > +#ifndef __UBOOT__
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#endif
> 
> Please drop this #if block
> 
> > +#include <linux/mtd/spinand.h>
> > +
> > +#define SPINAND_MFR_ETRON		0xD5
> > +
> > +#define STATUS_ECC_LIMIT_BITFLIPS	(3 << 4)
> 
> Please add a prefix aligned with Etron's namespace.

I'll use STATUS_ECC_MASK instead, as what the linux kernel does.

> 
> > +static SPINAND_OP_VARIANTS(read_cache_variants,
> > +		SPINAND_PAGE_READ_FROM_CACHE_1S_4S_4S_OP(0, 1, NULL, 0,
> > 0),
> > +		SPINAND_PAGE_READ_FROM_CACHE_1S_1S_4S_OP(0, 1, NULL, 0,
> > 0),
> > +		SPINAND_PAGE_READ_FROM_CACHE_1S_2S_2S_OP(0, 1, NULL, 0,
> > 0),
> > +		SPINAND_PAGE_READ_FROM_CACHE_1S_1S_2S_OP(0, 1, NULL, 0,
> > 0),
> > +		SPINAND_PAGE_READ_FROM_CACHE_FAST_1S_1S_1S_OP(0, 1,
> > NULL, 0, 0),
> > +		SPINAND_PAGE_READ_FROM_CACHE_1S_1S_1S_OP(0, 1, NULL, 0,
> > 0));
> > +
> > +static SPINAND_OP_VARIANTS(write_cache_variants,
> > +		SPINAND_PROG_LOAD_1S_1S_4S_OP(true, 0, NULL, 0),
> > +		SPINAND_PROG_LOAD_1S_1S_1S_OP(true, 0, NULL, 0));
> > +
> > +static SPINAND_OP_VARIANTS(update_cache_variants,
> > +		SPINAND_PROG_LOAD_1S_1S_4S_OP(false, 0, NULL, 0),
> > +		SPINAND_PROG_LOAD_1S_1S_1S_OP(false, 0, NULL, 0));
> > +
> > +static int etron_ecc4_ooblayout_ecc(struct mtd_info *mtd, int
> > section,
> > +				    struct mtd_oob_region *region)
> > +{
> > +	struct spinand_device *spinand = mtd_to_spinand(mtd);
> > +
> > +	if (section >= spinand->base.memorg.pagesize / mtd-
> > >ecc_step_size)
> > +		return -ERANGE;
> > +
> > +	region->offset = (8 * section) + 32;
> > +	region->length = 8;
> 
> This should be grouped into one big continuous area.

OK.

> 
> > +
> > +	return 0;
> > +}
> > +
> > +static int etron_ecc4_ooblayout_free(struct mtd_info *mtd, int
> > section,
> > +				     struct mtd_oob_region *region)
> > +{
> > +	struct spinand_device *spinand = mtd_to_spinand(mtd);
> > +
> > +	if (section >= spinand->base.memorg.pagesize / mtd-
> > >ecc_step_size)
> > +		return -ERANGE;
> > +
> > +	if (section) {
> > +		region->offset = 8 * section;
> > +		region->length = 8;
> 
> Ditto.
> 
> And same below, in the other OOB layouts.
> 
> > +	} else {
> > +		/* section 0 has two bytes reserved for bad block mark
> > */
> > +		region->offset = 2;
> > +		region->length = 6;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct mtd_ooblayout_ops etron_ecc4_ooblayout = {
> > +	.ecc = etron_ecc4_ooblayout_ecc,
> > +	.rfree = etron_ecc4_ooblayout_free,
> > +};
> > +
> > +static int etron_ecc8_ooblayout_ecc(struct mtd_info *mtd, int
> > section,
> > +				    struct mtd_oob_region *region)
> > +{
> > +	struct spinand_device *spinand = mtd_to_spinand(mtd);
> > +
> > +	if (section >= spinand->base.memorg.pagesize / mtd-
> > >ecc_step_size)
> > +		return -ERANGE;
> > +
> > +	region->offset = (14 * section) + 72;
> > +	region->length = 14;
> > +
> > +	return 0;
> > +}
> > +
> > +static int etron_ecc8_ooblayout_free(struct mtd_info *mtd, int
> > section,
> > +				     struct mtd_oob_region *region)
> > +{
> > +	struct spinand_device *spinand = mtd_to_spinand(mtd);
> > +
> > +	if (section >= spinand->base.memorg.pagesize / mtd-
> > >ecc_step_size)
> > +		return -ERANGE;
> > +
> > +	if (section) {
> > +		region->offset = 18 * section;
> > +		region->length = 18;
> > +	} else {
> > +		/* section 0 has two bytes reserved for bad block mark
> > */
> > +		region->offset = 2;
> > +		region->length = 16;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct mtd_ooblayout_ops etron_ecc8_ooblayout = {
> > +	.ecc = etron_ecc8_ooblayout_ecc,
> > +	.rfree = etron_ecc8_ooblayout_free,
> 
>          ^
> 
> Should not even compile?

I didn't see and mistake here.
This file can be compiled without error.

> 
> > +};
> > +
> > +static int etron_ecc_get_status(struct spinand_device *spinand, u8
> > status)
> > +{
> > +	struct nand_device *nand = spinand_to_nand(spinand);
> > +
> > +	switch (status & STATUS_ECC_MASK) {
> > +	case STATUS_ECC_NO_BITFLIPS:
> > +		return 0;
> > +
> > +	case STATUS_ECC_UNCOR_ERROR:
> > +		return -EBADMSG;
> > +
> > +	case STATUS_ECC_HAS_BITFLIPS:
> > +		return nand->eccreq.strength >> 1;
> 
> This is incorrect, you should probably return strength - 1 if that is
> what the bit means. We already had a discussion about this in the
> past:
> 
https://lore.kernel.org/linux-mtd/20210908201624.237634-1-bert@biot.com/
>  
> 

OK. I'll follow the linux kernel.

> > +
> > +	case STATUS_ECC_LIMIT_BITFLIPS:
> > +		return nand->eccreq.strength;
> > +
> > +	default:
> > +		break;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> 
> Thanks,
> Miquèl