[RFC PATCH 0/6] mtd: nand: per-partition ECC config

[RFC PATCH 0/6] mtd: nand: per-partition ECC config

[Boris Brezillon]

Hello,

It's been a year and a half since I posted my first series proposing
an approach to support per-partition ECC config [1].

First of all, before describing what's done in this patch series, I'd
like to sum-up why this is needed, and why a generic approach is
preferred over a NAND controller specific one.
On one side we have a lot of NAND chips out there and they all have
their own requirements in term of ECC strength and step size. On the
other side, most SoCs support booting from NAND (they embed a simple
logic in the ROM code to access a NAND chip through their NAND
controller).
In a ideal world all NAND chips would use the ONFI or JEDEC standard
exposing their requirements in a standard way, and the SoC vendors
would put the ONFI and JEDEC detection code in their ROM code and use
it to properly configure their NAND controller.
But we're not leaving in an ideal world, and some SoC vendors have
decided to hardcode (or use a simplified logic) to select the ECC
controller config. And in the case where the NAND requirement does
not match the ROM code config, you only have two solutions:

1/ leave with the unsuitable ECC config for the whole chip
2/ isolate the portion of NAND read by the ROM code into a sperate
   partition and use a suitable ECC config for the rest of the
   NAND

IMHO the second solution is far better than the first one, but it
requires some adjustments in the mtdpart and NAND code layer to be
applicable.

Now, why should we prefer a generic approach over a NAND controller/SoC
specific one ?
Because, this seems to be a problem faced by other people on other
platform than the sunxi one. Moreover, the ECC config is not the only
thing we'd have to tweak per partition: I'm currently working on the
NAND randomizer/scrambler aspect (required to support some MLC chips),
and this is also something the ROM code configure differently to
boot the first stage bootloader.
For all these reasons, I think providing a generic infrastructure allowing
specific implementation to tweak their behavior is better than hardcoding
it somewhere in the NAND controller driver.

This series proposes a solution to allow such per-partition config by
first letting MTD implementations (or subframework) overload the MTD
partition functions (patches 1 and 2), and then providing the appropriate
modifications in the NAND layer to support per-partition ECC config
(patches 3 to 5).
The last patch is showing how a NAND controller can add support for
per-partition ECC config.

Note that I tried to keep the changes as less invasive as impossible, but
I might have missed some aspects.
Any suggestions are welcome.

Best Regards,

Boris

[1]https://lkml.org/lkml/2014/2/8/73

Boris Brezillon (6):
  mtd: allow device-specific partition handling
  mtd: part: pass OF node to the MTD partition layer
  mtd: nand: move nand_ecc_ctrl initialization out of nand_scan_tail
  mtd: nand: add an helper to access the ecc controller struct
  mtd: nand: add infrastructure for per-partition ECC config
  mtd: nand: sunxi: support per-partition ECC config

 drivers/mtd/mtdpart.c          |  26 +-
 drivers/mtd/nand/nand_base.c   | 526 ++++++++++++++++++++++++++++-------------
 drivers/mtd/nand/sunxi_nand.c  |  61 ++++-
 drivers/mtd/ofpart.c           |   1 +
 include/linux/mtd/mtd.h        |  45 ++++
 include/linux/mtd/nand.h       |  38 +++
 include/linux/mtd/partitions.h |   3 +-
 7 files changed, 519 insertions(+), 181 deletions(-)

-- 
1.9.1

[RFC PATCH 1/6] mtd: allow device-specific partition handling

[Boris Brezillon]

Currently, only the mtdpart implementation is allowed to expose MTD device
partitions, but some devices might need specific handling for special
partitions.
This is particularly true for systems where the NAND device is used as a
boot media. On such systems, the ROM code only support a subset of ECC
modes, and this subset may not match the chip requirements. In this
particular case we only have two options:
- force the whole chip to use one of the ECC mode supported by the ROM code
- allow for per-partition ECC setting
The first solution should be avoided if the ECC setting used by the ROM
code are weaker than the NAND requirements, hence

Add a generic way to overload the default partition handling so that
MTD drivers (or sub-frameworks) can implement their own methods.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/mtdpart.c   | 24 ++++++++++++++++--------
 include/linux/mtd/mtd.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 3dc479f..9a45230 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -38,14 +38,6 @@
 static LIST_HEAD(mtd_partitions);
 static DEFINE_MUTEX(mtd_partitions_mutex);
 
-/* Our partition node structure */
-struct mtd_part {
-	struct mtd_info mtd;
-	struct mtd_info *master;
-	uint64_t offset;
-	struct list_head list;
-};
-
 /*
  * Given a pointer to the MTD object in the mtd_part structure, we can retrieve
  * the pointer to that structure with this macro.
@@ -319,6 +311,9 @@ static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
 
 static inline void free_partition(struct mtd_part *p)
 {
+	if (p->master->part_ops && p->master->part_ops->remove)
+		p->master->part_ops->remove(p);
+
 	kfree(p->mtd.name);
 	kfree(p);
 }
@@ -551,6 +546,19 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
 		}
 	}
 
+	if (master->part_ops && master->part_ops->add) {
+		int ret;
+
+		ret = master->part_ops->add(slave);
+		if (ret) {
+			pr_err("error %d while creating partitions for \"%s\"\n",
+			       ret, master->name);
+			kfree(name);
+			kfree(slave);
+			return ERR_PTR(ret);
+		}
+	}
+
 out_register:
 	return slave;
 }
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index f17fa75..17f8688 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -111,6 +111,8 @@ struct nand_ecclayout {
 
 struct module;	/* only needed for owner field in mtd_info */
 
+struct mtd_part_ops;
+
 struct mtd_info {
 	u_char type;
 	uint32_t flags;
@@ -235,6 +237,8 @@ struct mtd_info {
 	int (*_get_device) (struct mtd_info *mtd);
 	void (*_put_device) (struct mtd_info *mtd);
 
+	const struct mtd_part_ops *part_ops;
+
 	/* Backing device capabilities for this device
 	 * - provides mmap capabilities
 	 */
@@ -254,6 +258,47 @@ struct mtd_info {
 	int usecount;
 };
 
+/**
+ * struct mtd_part - MTD partition structure
+ * @mtd:	MTD partition device
+ * @master:	MTD master device
+ * @offset:	start offset
+ * @list:	list node
+ */
+struct mtd_part {
+	struct mtd_info mtd;
+	struct mtd_info *master;
+	uint64_t offset;
+	struct list_head list;
+};
+
+static inline struct mtd_part *mtd_to_part(struct mtd_info *mtd)
+{
+	return container_of(mtd, struct mtd_part, mtd);
+}
+
+static inline void *mtd_part_get_priv(struct mtd_part *part)
+{
+	return part->mtd.priv;
+}
+
+static inline void mtd_part_set_priv(struct mtd_part *part, void *priv)
+{
+	part->mtd.priv = priv;
+}
+
+/**
+ * struct mtd_part_ops - MTD partition operations
+ * @add: add a new MTD partition and instantiate the associated data.
+ *	 You should overload the MTD callbacks if you want a specific
+ *	 behavior.
+ * @remove: remove an existing MTD partition
+ */
+struct mtd_part_ops {
+	int (*add)(struct mtd_part *part);
+	void (*remove)(struct mtd_part *part);
+};
+
 int mtd_erase(struct mtd_info *mtd, struct erase_info *instr);
 int mtd_point(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
 	      void **virt, resource_size_t *phys);
-- 
1.9.1

[RFC PATCH 2/6] mtd: part: pass OF node to the MTD partition layer

[Boris Brezillon]

Pass the OF node attached an MTD partition so that specific MTD device
implementations can extract extra information from the DT definition.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/mtdpart.c          | 2 ++
 drivers/mtd/ofpart.c           | 1 +
 include/linux/mtd/partitions.h | 3 ++-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 9a45230..c2e5920 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -362,6 +362,8 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
 		return ERR_PTR(-ENOMEM);
 	}
 
+	slave->mtd.dev.of_node = part->of_node;
+
 	/* set up the MTD object for this partition */
 	slave->mtd.type = master->type;
 	slave->mtd.flags = master->flags & ~part->mask_flags;
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index aa26c32..8a06cfb 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -73,6 +73,7 @@ static int parse_ofpart_partitions(struct mtd_info *master,
 			continue;
 		}
 
+		(*pparts)[i].of_node = pp;
 		a_cells = of_n_addr_cells(pp);
 		s_cells = of_n_size_cells(pp);
 		(*pparts)[i].offset = of_read_number(reg, a_cells);
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 6a35e6d..e4ff8a3 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -12,6 +12,7 @@
 #include <linux/types.h>
 
 
+
 /*
  * Partition definition structure:
  *
@@ -42,6 +43,7 @@ struct mtd_partition {
 	uint64_t offset;		/* offset within the master MTD space */
 	uint32_t mask_flags;		/* master MTD flags to mask out for this partition */
 	struct nand_ecclayout *ecclayout;	/* out of band layout for this partition (NAND only) */
+	struct device_node *of_node;	/* OF node attached to this partition (optional) */
 };
 
 #define MTDPART_OFS_RETAIN	(-3)
@@ -63,7 +65,6 @@ struct mtd_part_parser_data {
 	struct device_node *of_node;
 };
 
-
 /*
  * Functions dealing with the various ways of partitioning the space
  */
-- 
1.9.1

[RFC PATCH 3/6] mtd: nand: move nand_ecc_ctrl initialization out of nand_scan_tail

[Boris Brezillon]

The ECC initialization is currently done when the NAND chip is
registered to the NAND framework, but we are about to add per-partition
ECC setting.
Move the ECC initialization code into its own function so that it can be
reused for the per-partition config.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 104 ++++++++++++++++++++++++++-----------------
 1 file changed, 64 insertions(+), 40 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ceb68ca..0321126 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3928,43 +3928,14 @@ static bool nand_ecc_strength_good(struct mtd_info *mtd)
 	return corr >= ds_corr && ecc->strength >= chip->ecc_strength_ds;
 }
 
-/**
- * nand_scan_tail - [NAND Interface] Scan for the NAND device
- * @mtd: MTD device structure
- *
- * This is the second phase of the normal nand_scan() function. It fills out
- * all the uninitialized function pointers with the defaults and scans for a
- * bad block table if appropriate.
+/*
+ * Initialize ECC struct:
+ *  - fill ECC struct with default function/values when these ones are undefined
+ *  - fill ECC infos based on MTD device
  */
-int nand_scan_tail(struct mtd_info *mtd)
+static int nand_ecc_ctrl_init(struct mtd_info *mtd, struct nand_ecc_ctrl *ecc)
 {
 	int i;
-	struct nand_chip *chip = mtd->priv;
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
-	struct nand_buffers *nbuf;
-
-	/* New bad blocks should be marked in OOB, flash-based BBT, or both */
-	BUG_ON((chip->bbt_options & NAND_BBT_NO_OOB_BBM) &&
-			!(chip->bbt_options & NAND_BBT_USE_FLASH));
-
-	if (!(chip->options & NAND_OWN_BUFFERS)) {
-		nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
-				+ mtd->oobsize * 3, GFP_KERNEL);
-		if (!nbuf)
-			return -ENOMEM;
-		nbuf->ecccalc = (uint8_t *)(nbuf + 1);
-		nbuf->ecccode = nbuf->ecccalc + mtd->oobsize;
-		nbuf->databuf = nbuf->ecccode + mtd->oobsize;
-
-		chip->buffers = nbuf;
-	} else {
-		if (!chip->buffers)
-			return -ENOMEM;
-	}
-
-	/* Set the internal oob buffer location, just after the page data */
-	chip->oob_poi = chip->buffers->databuf + mtd->writesize;
-
 	/*
 	 * If no default placement scheme is given, select an appropriate one.
 	 */
@@ -3989,9 +3960,6 @@ int nand_scan_tail(struct mtd_info *mtd)
 		}
 	}
 
-	if (!chip->write_page)
-		chip->write_page = nand_write_page;
-
 	/*
 	 * Check ECC mode, default to software if 3byte/512byte hardware ECC is
 	 * selected and we have 256 byte pagesize fallback to software ECC
@@ -4161,6 +4129,64 @@ int nand_scan_tail(struct mtd_info *mtd)
 	}
 	ecc->total = ecc->steps * ecc->bytes;
 
+	return 0;
+}
+
+static void nand_ecc_cleanup(struct mtd_info *mtd, struct nand_ecc_ctrl *ecc)
+{
+	if (ecc->mode == NAND_ECC_SOFT_BCH)
+		nand_bch_free((struct nand_bch_control *)ecc->priv);
+}
+
+/**
+ * nand_scan_tail - [NAND Interface] Scan for the NAND device
+ * @mtd: MTD device structure
+ *
+ * This is the second phase of the normal nand_scan() function. It fills out
+ * all the uninitialized function pointers with the defaults and scans for a
+ * bad block table if appropriate.
+ */
+int nand_scan_tail(struct mtd_info *mtd)
+{
+	int ret;
+	struct nand_chip *chip = mtd->priv;
+	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct nand_buffers *nbuf;
+
+	/* New bad blocks should be marked in OOB, flash-based BBT, or both */
+	BUG_ON((chip->bbt_options & NAND_BBT_NO_OOB_BBM) &&
+			!(chip->bbt_options & NAND_BBT_USE_FLASH));
+
+	if (!(chip->options & NAND_OWN_BUFFERS)) {
+		nbuf = kzalloc(sizeof(*nbuf) + mtd->writesize
+				+ mtd->oobsize * 3, GFP_KERNEL);
+		if (!nbuf)
+			return -ENOMEM;
+		nbuf->ecccalc = (uint8_t *)(nbuf + 1);
+		nbuf->ecccode = nbuf->ecccalc + mtd->oobsize;
+		nbuf->databuf = nbuf->ecccode + mtd->oobsize;
+
+		chip->buffers = nbuf;
+	} else {
+		if (!chip->buffers)
+			return -ENOMEM;
+	}
+
+	/* Set the internal oob buffer location, just after the page data */
+	chip->oob_poi = chip->buffers->databuf + mtd->writesize;
+
+	if (!chip->write_page)
+		chip->write_page = nand_write_page;
+
+	/* Initialize ECC struct */
+	ret = nand_ecc_ctrl_init(mtd, ecc);
+	if (ret) {
+		if (!(chip->options & NAND_OWN_BUFFERS))
+			kfree(chip->buffers);
+
+		return ret;
+	}
+
 	/* Allow subpage writes up to ecc.steps. Not possible for MLC flash */
 	if (!(chip->options & NAND_NO_SUBPAGE_WRITE) && nand_is_slc(chip)) {
 		switch (ecc->steps) {
@@ -4285,9 +4311,7 @@ void nand_release(struct mtd_info *mtd)
 {
 	struct nand_chip *chip = mtd->priv;
 
-	if (chip->ecc.mode == NAND_ECC_SOFT_BCH)
-		nand_bch_free((struct nand_bch_control *)chip->ecc.priv);
-
+	nand_ecc_cleanup(mtd, &chip->ecc);
 	mtd_device_unregister(mtd);
 
 	/* Free bad block table memory */
-- 
1.9.1

[RFC PATCH 4/6] mtd: nand: add an helper to access the ecc controller struct

[Boris Brezillon]

The ecc controller structure is currently directly accessed (chip->ecc),
but since we are about to add support for per-partition ECC, providing
an helper function to access this field will make the migration easier.

Create the nand_ecc() helper and modify all direct accesses to the ecc
field in the NAND core code.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 270 +++++++++++++++++++++++--------------------
 include/linux/mtd/nand.h     |   5 +
 2 files changed, 147 insertions(+), 128 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 0321126..67a29f5 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1133,26 +1133,26 @@ static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
 				       struct nand_chip *chip, uint8_t *buf,
 				       int oob_required, int page)
 {
-	int eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
+	int eccsize = nand_ecc(chip)->size;
+	int eccbytes = nand_ecc(chip)->bytes;
 	uint8_t *oob = chip->oob_poi;
 	int steps, size;
 
-	for (steps = chip->ecc.steps; steps > 0; steps--) {
+	for (steps = nand_ecc(chip)->steps; steps > 0; steps--) {
 		chip->read_buf(mtd, buf, eccsize);
 		buf += eccsize;
 
-		if (chip->ecc.prepad) {
-			chip->read_buf(mtd, oob, chip->ecc.prepad);
-			oob += chip->ecc.prepad;
+		if (nand_ecc(chip)->prepad) {
+			chip->read_buf(mtd, oob, nand_ecc(chip)->prepad);
+			oob += nand_ecc(chip)->prepad;
 		}
 
 		chip->read_buf(mtd, oob, eccbytes);
 		oob += eccbytes;
 
-		if (chip->ecc.postpad) {
-			chip->read_buf(mtd, oob, chip->ecc.postpad);
-			oob += chip->ecc.postpad;
+		if (nand_ecc(chip)->postpad) {
+			chip->read_buf(mtd, oob, nand_ecc(chip)->postpad);
+			oob += nand_ecc(chip)->postpad;
 		}
 	}
 
@@ -1174,30 +1174,31 @@ static int nand_read_page_raw_syndrome(struct mtd_info *mtd,
 static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
 				uint8_t *buf, int oob_required, int page)
 {
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
+	int i, eccsize = nand_ecc(chip)->size;
+	int eccbytes = nand_ecc(chip)->bytes;
+	int eccsteps = nand_ecc(chip)->steps;
 	uint8_t *p = buf;
 	uint8_t *ecc_calc = chip->buffers->ecccalc;
 	uint8_t *ecc_code = chip->buffers->ecccode;
-	uint32_t *eccpos = chip->ecc.layout->eccpos;
+	uint32_t *eccpos = nand_ecc(chip)->layout->eccpos;
 	unsigned int max_bitflips = 0;
 
-	chip->ecc.read_page_raw(mtd, chip, buf, 1, page);
+	nand_ecc(chip)->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]);
+		nand_ecc(chip)->calculate(mtd, p, &ecc_calc[i]);
 
-	for (i = 0; i < chip->ecc.total; i++)
+	for (i = 0; i < nand_ecc(chip)->total; i++)
 		ecc_code[i] = chip->oob_poi[eccpos[i]];
 
-	eccsteps = chip->ecc.steps;
+	eccsteps = nand_ecc(chip)->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]);
+		stat = nand_ecc(chip)->correct(mtd, p, &ecc_code[i],
+					       &ecc_calc[i]);
 		if (stat < 0) {
 			mtd->ecc_stats.failed++;
 		} else {
@@ -1222,7 +1223,7 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 			int page)
 {
 	int start_step, end_step, num_steps;
-	uint32_t *eccpos = chip->ecc.layout->eccpos;
+	uint32_t *eccpos = nand_ecc(chip)->layout->eccpos;
 	uint8_t *p;
 	int data_col_addr, i, gaps = 0;
 	int datafrag_len, eccfrag_len, aligned_len, aligned_pos;
@@ -1231,16 +1232,16 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 	unsigned int max_bitflips = 0;
 
 	/* Column address within the page aligned to ECC size (256bytes) */
-	start_step = data_offs / chip->ecc.size;
-	end_step = (data_offs + readlen - 1) / chip->ecc.size;
+	start_step = data_offs / nand_ecc(chip)->size;
+	end_step = (data_offs + readlen - 1) / nand_ecc(chip)->size;
 	num_steps = end_step - start_step + 1;
-	index = start_step * chip->ecc.bytes;
+	index = start_step * nand_ecc(chip)->bytes;
 
 	/* Data size aligned to ECC ecc.size */
-	datafrag_len = num_steps * chip->ecc.size;
-	eccfrag_len = num_steps * chip->ecc.bytes;
+	datafrag_len = num_steps * nand_ecc(chip)->size;
+	eccfrag_len = num_steps * nand_ecc(chip)->bytes;
 
-	data_col_addr = start_step * chip->ecc.size;
+	data_col_addr = start_step * nand_ecc(chip)->size;
 	/* If we read not a page aligned data */
 	if (data_col_addr != 0)
 		chip->cmdfunc(mtd, NAND_CMD_RNDOUT, data_col_addr, -1);
@@ -1249,8 +1250,9 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 	chip->read_buf(mtd, p, datafrag_len);
 
 	/* Calculate ECC */
-	for (i = 0; i < eccfrag_len ; i += chip->ecc.bytes, p += chip->ecc.size)
-		chip->ecc.calculate(mtd, p, &chip->buffers->ecccalc[i]);
+	for (i = 0; i < eccfrag_len;
+	     i += nand_ecc(chip)->bytes, p += nand_ecc(chip)->size)
+		nand_ecc(chip)->calculate(mtd, p, &chip->buffers->ecccalc[i]);
 
 	/*
 	 * The performance is faster if we position offsets according to
@@ -1274,7 +1276,8 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 		aligned_len = eccfrag_len;
 		if (eccpos[index] & (busw - 1))
 			aligned_len++;
-		if (eccpos[index + (num_steps * chip->ecc.bytes)] & (busw - 1))
+		if (eccpos[index + (num_steps * nand_ecc(chip)->bytes)] &
+		    (busw - 1))
 			aligned_len++;
 
 		chip->cmdfunc(mtd, NAND_CMD_RNDOUT,
@@ -1286,11 +1289,13 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 		chip->buffers->ecccode[i] = chip->oob_poi[eccpos[i + index]];
 
 	p = bufpoi + data_col_addr;
-	for (i = 0; i < eccfrag_len ; i += chip->ecc.bytes, p += chip->ecc.size) {
+	for (i = 0; i < eccfrag_len;
+	     i += nand_ecc(chip)->bytes, p += nand_ecc(chip)->size) {
 		int stat;
 
-		stat = chip->ecc.correct(mtd, p,
-			&chip->buffers->ecccode[i], &chip->buffers->ecccalc[i]);
+		stat = nand_ecc(chip)->correct(mtd, p,
+			&chip->buffers->ecccode[i],
+			&chip->buffers->ecccalc[i]);
 		if (stat < 0) {
 			mtd->ecc_stats.failed++;
 		} else {
@@ -1314,32 +1319,33 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 				uint8_t *buf, int oob_required, int page)
 {
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
+	int i, eccsize = nand_ecc(chip)->size;
+	int eccbytes = nand_ecc(chip)->bytes;
+	int eccsteps = nand_ecc(chip)->steps;
 	uint8_t *p = buf;
 	uint8_t *ecc_calc = chip->buffers->ecccalc;
 	uint8_t *ecc_code = chip->buffers->ecccode;
-	uint32_t *eccpos = chip->ecc.layout->eccpos;
+	uint32_t *eccpos = nand_ecc(chip)->layout->eccpos;
 	unsigned int max_bitflips = 0;
 
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
-		chip->ecc.hwctl(mtd, NAND_ECC_READ);
+		nand_ecc(chip)->hwctl(mtd, NAND_ECC_READ);
 		chip->read_buf(mtd, p, eccsize);
-		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
+		nand_ecc(chip)->calculate(mtd, p, &ecc_calc[i]);
 	}
 	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
 
-	for (i = 0; i < chip->ecc.total; i++)
+	for (i = 0; i < nand_ecc(chip)->total; i++)
 		ecc_code[i] = chip->oob_poi[eccpos[i]];
 
-	eccsteps = chip->ecc.steps;
+	eccsteps = nand_ecc(chip)->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]);
+		stat = nand_ecc(chip)->correct(mtd, p, &ecc_code[i],
+					       &ecc_calc[i]);
 		if (stat < 0) {
 			mtd->ecc_stats.failed++;
 		} else {
@@ -1367,12 +1373,12 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
 	struct nand_chip *chip, uint8_t *buf, int oob_required, int page)
 {
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
+	int i, eccsize = nand_ecc(chip)->size;
+	int eccbytes = nand_ecc(chip)->bytes;
+	int eccsteps = nand_ecc(chip)->steps;
 	uint8_t *p = buf;
 	uint8_t *ecc_code = chip->buffers->ecccode;
-	uint32_t *eccpos = chip->ecc.layout->eccpos;
+	uint32_t *eccpos = nand_ecc(chip)->layout->eccpos;
 	uint8_t *ecc_calc = chip->buffers->ecccalc;
 	unsigned int max_bitflips = 0;
 
@@ -1381,17 +1387,17 @@ static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
 	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
 	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
 
-	for (i = 0; i < chip->ecc.total; i++)
+	for (i = 0; i < nand_ecc(chip)->total; i++)
 		ecc_code[i] = chip->oob_poi[eccpos[i]];
 
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 		int stat;
 
-		chip->ecc.hwctl(mtd, NAND_ECC_READ);
+		nand_ecc(chip)->hwctl(mtd, NAND_ECC_READ);
 		chip->read_buf(mtd, p, eccsize);
-		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
+		nand_ecc(chip)->calculate(mtd, p, &ecc_calc[i]);
 
-		stat = chip->ecc.correct(mtd, p, &ecc_code[i], NULL);
+		stat = nand_ecc(chip)->correct(mtd, p, &ecc_code[i], NULL);
 		if (stat < 0) {
 			mtd->ecc_stats.failed++;
 		} else {
@@ -1416,9 +1422,9 @@ static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
 static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
 				   uint8_t *buf, int oob_required, int page)
 {
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
+	int i, eccsize = nand_ecc(chip)->size;
+	int eccbytes = nand_ecc(chip)->bytes;
+	int eccsteps = nand_ecc(chip)->steps;
 	uint8_t *p = buf;
 	uint8_t *oob = chip->oob_poi;
 	unsigned int max_bitflips = 0;
@@ -1426,17 +1432,17 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 		int stat;
 
-		chip->ecc.hwctl(mtd, NAND_ECC_READ);
+		nand_ecc(chip)->hwctl(mtd, NAND_ECC_READ);
 		chip->read_buf(mtd, p, eccsize);
 
-		if (chip->ecc.prepad) {
-			chip->read_buf(mtd, oob, chip->ecc.prepad);
-			oob += chip->ecc.prepad;
+		if (nand_ecc(chip)->prepad) {
+			chip->read_buf(mtd, oob, nand_ecc(chip)->prepad);
+			oob += nand_ecc(chip)->prepad;
 		}
 
-		chip->ecc.hwctl(mtd, NAND_ECC_READSYN);
+		nand_ecc(chip)->hwctl(mtd, NAND_ECC_READSYN);
 		chip->read_buf(mtd, oob, eccbytes);
-		stat = chip->ecc.correct(mtd, p, oob, NULL);
+		stat = nand_ecc(chip)->correct(mtd, p, oob, NULL);
 
 		if (stat < 0) {
 			mtd->ecc_stats.failed++;
@@ -1447,9 +1453,9 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
 
 		oob += eccbytes;
 
-		if (chip->ecc.postpad) {
-			chip->read_buf(mtd, oob, chip->ecc.postpad);
-			oob += chip->ecc.postpad;
+		if (nand_ecc(chip)->postpad) {
+			chip->read_buf(mtd, oob, nand_ecc(chip)->postpad);
+			oob += nand_ecc(chip)->postpad;
 		}
 	}
 
@@ -1479,7 +1485,7 @@ static uint8_t *nand_transfer_oob(struct nand_chip *chip, uint8_t *oob,
 		return oob + len;
 
 	case MTD_OPS_AUTO_OOB: {
-		struct nand_oobfree *free = chip->ecc.layout->oobfree;
+		struct nand_oobfree *free = nand_ecc(chip)->layout->oobfree;
 		uint32_t boffs = 0, roffs = ops->ooboffs;
 		size_t bytes = 0;
 
@@ -1599,16 +1605,18 @@ read_retry:
 			 * the read methods return max bitflips per ecc step.
 			 */
 			if (unlikely(ops->mode == MTD_OPS_RAW))
-				ret = chip->ecc.read_page_raw(mtd, chip, bufpoi,
+				ret = nand_ecc(chip)->read_page_raw(mtd, chip,
+							      bufpoi,
 							      oob_required,
 							      page);
 			else if (!aligned && NAND_HAS_SUBPAGE_READ(chip) &&
 				 !oob)
-				ret = chip->ecc.read_subpage(mtd, chip,
+				ret = nand_ecc(chip)->read_subpage(mtd, chip,
 							col, bytes, bufpoi,
 							page);
 			else
-				ret = chip->ecc.read_page(mtd, chip, bufpoi,
+				ret = nand_ecc(chip)->read_page(mtd, chip,
+							  bufpoi,
 							  oob_required, page);
 			if (ret < 0) {
 				if (use_bufpoi)
@@ -1769,13 +1777,14 @@ static int nand_read_oob_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
 				  int page)
 {
 	int length = mtd->oobsize;
-	int chunk = chip->ecc.bytes + chip->ecc.prepad + chip->ecc.postpad;
-	int eccsize = chip->ecc.size;
+	int chunk = nand_ecc(chip)->bytes + nand_ecc(chip)->prepad +
+		    nand_ecc(chip)->postpad;
+	int eccsize = nand_ecc(chip)->size;
 	uint8_t *bufpoi = chip->oob_poi;
 	int i, toread, sndrnd = 0, pos;
 
-	chip->cmdfunc(mtd, NAND_CMD_READ0, chip->ecc.size, page);
-	for (i = 0; i < chip->ecc.steps; i++) {
+	chip->cmdfunc(mtd, NAND_CMD_READ0, nand_ecc(chip)->size, page);
+	for (i = 0; i < nand_ecc(chip)->steps; i++) {
 		if (sndrnd) {
 			pos = eccsize + i * (eccsize + chunk);
 			if (mtd->writesize > 512)
@@ -1828,9 +1837,10 @@ static int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip,
 static int nand_write_oob_syndrome(struct mtd_info *mtd,
 				   struct nand_chip *chip, int page)
 {
-	int chunk = chip->ecc.bytes + chip->ecc.prepad + chip->ecc.postpad;
-	int eccsize = chip->ecc.size, length = mtd->oobsize;
-	int i, len, pos, status = 0, sndcmd = 0, steps = chip->ecc.steps;
+	int chunk = nand_ecc(chip)->bytes + nand_ecc(chip)->prepad +
+		    nand_ecc(chip)->postpad;
+	int eccsize = nand_ecc(chip)->size, length = mtd->oobsize;
+	int i, len, pos, status = 0, sndcmd = 0, steps = nand_ecc(chip)->steps;
 	const uint8_t *bufpoi = chip->oob_poi;
 
 	/*
@@ -1838,7 +1848,7 @@ static int nand_write_oob_syndrome(struct mtd_info *mtd,
 	 * or
 	 * data-pad-ecc-pad-data-pad .... ecc-pad-oob
 	 */
-	if (!chip->ecc.prepad && !chip->ecc.postpad) {
+	if (!nand_ecc(chip)->prepad && !nand_ecc(chip)->postpad) {
 		pos = steps * (eccsize + chunk);
 		steps = 0;
 	} else
@@ -1902,7 +1912,7 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from,
 	stats = mtd->ecc_stats;
 
 	if (ops->mode == MTD_OPS_AUTO_OOB)
-		len = chip->ecc.layout->oobavail;
+		len = nand_ecc(chip)->layout->oobavail;
 	else
 		len = mtd->oobsize;
 
@@ -1930,9 +1940,9 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from,
 
 	while (1) {
 		if (ops->mode == MTD_OPS_RAW)
-			ret = chip->ecc.read_oob_raw(mtd, chip, page);
+			ret = nand_ecc(chip)->read_oob_raw(mtd, chip, page);
 		else
-			ret = chip->ecc.read_oob(mtd, chip, page);
+			ret = nand_ecc(chip)->read_oob(mtd, chip, page);
 
 		if (ret < 0)
 			break;
@@ -2053,26 +2063,26 @@ static int nand_write_page_raw_syndrome(struct mtd_info *mtd,
 					struct nand_chip *chip,
 					const uint8_t *buf, int oob_required)
 {
-	int eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
+	int eccsize = nand_ecc(chip)->size;
+	int eccbytes = nand_ecc(chip)->bytes;
 	uint8_t *oob = chip->oob_poi;
 	int steps, size;
 
-	for (steps = chip->ecc.steps; steps > 0; steps--) {
+	for (steps = nand_ecc(chip)->steps; steps > 0; steps--) {
 		chip->write_buf(mtd, buf, eccsize);
 		buf += eccsize;
 
-		if (chip->ecc.prepad) {
-			chip->write_buf(mtd, oob, chip->ecc.prepad);
-			oob += chip->ecc.prepad;
+		if (nand_ecc(chip)->prepad) {
+			chip->write_buf(mtd, oob, nand_ecc(chip)->prepad);
+			oob += nand_ecc(chip)->prepad;
 		}
 
 		chip->write_buf(mtd, oob, eccbytes);
 		oob += eccbytes;
 
-		if (chip->ecc.postpad) {
-			chip->write_buf(mtd, oob, chip->ecc.postpad);
-			oob += chip->ecc.postpad;
+		if (nand_ecc(chip)->postpad) {
+			chip->write_buf(mtd, oob, nand_ecc(chip)->postpad);
+			oob += nand_ecc(chip)->postpad;
 		}
 	}
 
@@ -2092,21 +2102,21 @@ static int nand_write_page_raw_syndrome(struct mtd_info *mtd,
 static int nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
 				  const uint8_t *buf, int oob_required)
 {
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
+	int i, eccsize = nand_ecc(chip)->size;
+	int eccbytes = nand_ecc(chip)->bytes;
+	int eccsteps = nand_ecc(chip)->steps;
 	uint8_t *ecc_calc = chip->buffers->ecccalc;
 	const uint8_t *p = buf;
-	uint32_t *eccpos = chip->ecc.layout->eccpos;
+	uint32_t *eccpos = nand_ecc(chip)->layout->eccpos;
 
 	/* Software ECC calculation */
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
-		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
+		nand_ecc(chip)->calculate(mtd, p, &ecc_calc[i]);
 
-	for (i = 0; i < chip->ecc.total; i++)
+	for (i = 0; i < nand_ecc(chip)->total; i++)
 		chip->oob_poi[eccpos[i]] = ecc_calc[i];
 
-	return chip->ecc.write_page_raw(mtd, chip, buf, 1);
+	return nand_ecc(chip)->write_page_raw(mtd, chip, buf, 1);
 }
 
 /**
@@ -2119,20 +2129,20 @@ static int nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
 static int nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 				  const uint8_t *buf, int oob_required)
 {
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
+	int i, eccsize = nand_ecc(chip)->size;
+	int eccbytes = nand_ecc(chip)->bytes;
+	int eccsteps = nand_ecc(chip)->steps;
 	uint8_t *ecc_calc = chip->buffers->ecccalc;
 	const uint8_t *p = buf;
-	uint32_t *eccpos = chip->ecc.layout->eccpos;
+	uint32_t *eccpos = nand_ecc(chip)->layout->eccpos;
 
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
-		chip->ecc.hwctl(mtd, NAND_ECC_WRITE);
+		nand_ecc(chip)->hwctl(mtd, NAND_ECC_WRITE);
 		chip->write_buf(mtd, p, eccsize);
-		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
+		nand_ecc(chip)->calculate(mtd, p, &ecc_calc[i]);
 	}
 
-	for (i = 0; i < chip->ecc.total; i++)
+	for (i = 0; i < nand_ecc(chip)->total; i++)
 		chip->oob_poi[eccpos[i]] = ecc_calc[i];
 
 	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
@@ -2157,10 +2167,10 @@ static int nand_write_subpage_hwecc(struct mtd_info *mtd,
 {
 	uint8_t *oob_buf  = chip->oob_poi;
 	uint8_t *ecc_calc = chip->buffers->ecccalc;
-	int ecc_size      = chip->ecc.size;
-	int ecc_bytes     = chip->ecc.bytes;
-	int ecc_steps     = chip->ecc.steps;
-	uint32_t *eccpos  = chip->ecc.layout->eccpos;
+	int ecc_size      = nand_ecc(chip)->size;
+	int ecc_bytes     = nand_ecc(chip)->bytes;
+	int ecc_steps     = nand_ecc(chip)->steps;
+	uint32_t *eccpos  = nand_ecc(chip)->layout->eccpos;
 	uint32_t start_step = offset / ecc_size;
 	uint32_t end_step   = (offset + data_len - 1) / ecc_size;
 	int oob_bytes       = mtd->oobsize / ecc_steps;
@@ -2168,7 +2178,7 @@ static int nand_write_subpage_hwecc(struct mtd_info *mtd,
 
 	for (step = 0; step < ecc_steps; step++) {
 		/* configure controller for WRITE access */
-		chip->ecc.hwctl(mtd, NAND_ECC_WRITE);
+		nand_ecc(chip)->hwctl(mtd, NAND_ECC_WRITE);
 
 		/* write data (untouched subpages already masked by 0xFF) */
 		chip->write_buf(mtd, buf, ecc_size);
@@ -2177,7 +2187,7 @@ static int nand_write_subpage_hwecc(struct mtd_info *mtd,
 		if ((step < start_step) || (step > end_step))
 			memset(ecc_calc, 0xff, ecc_bytes);
 		else
-			chip->ecc.calculate(mtd, buf, ecc_calc);
+			nand_ecc(chip)->calculate(mtd, buf, ecc_calc);
 
 		/* mask OOB of un-touched subpages by padding 0xFF */
 		/* if oob_required, preserve OOB metadata of written subpage */
@@ -2192,7 +2202,7 @@ static int nand_write_subpage_hwecc(struct mtd_info *mtd,
 	/* copy calculated ECC for whole page to chip->buffer->oob */
 	/* this include masked-value(0xFF) for unwritten subpages */
 	ecc_calc = chip->buffers->ecccalc;
-	for (i = 0; i < chip->ecc.total; i++)
+	for (i = 0; i < nand_ecc(chip)->total; i++)
 		chip->oob_poi[eccpos[i]] = ecc_calc[i];
 
 	/* write OOB buffer to NAND device */
@@ -2216,29 +2226,29 @@ static int nand_write_page_syndrome(struct mtd_info *mtd,
 				    struct nand_chip *chip,
 				    const uint8_t *buf, int oob_required)
 {
-	int i, eccsize = chip->ecc.size;
-	int eccbytes = chip->ecc.bytes;
-	int eccsteps = chip->ecc.steps;
+	int i, eccsize = nand_ecc(chip)->size;
+	int eccbytes = nand_ecc(chip)->bytes;
+	int eccsteps = nand_ecc(chip)->steps;
 	const uint8_t *p = buf;
 	uint8_t *oob = chip->oob_poi;
 
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 
-		chip->ecc.hwctl(mtd, NAND_ECC_WRITE);
+		nand_ecc(chip)->hwctl(mtd, NAND_ECC_WRITE);
 		chip->write_buf(mtd, p, eccsize);
 
-		if (chip->ecc.prepad) {
-			chip->write_buf(mtd, oob, chip->ecc.prepad);
-			oob += chip->ecc.prepad;
+		if (nand_ecc(chip)->prepad) {
+			chip->write_buf(mtd, oob, nand_ecc(chip)->prepad);
+			oob += nand_ecc(chip)->prepad;
 		}
 
-		chip->ecc.calculate(mtd, p, oob);
+		nand_ecc(chip)->calculate(mtd, p, oob);
 		chip->write_buf(mtd, oob, eccbytes);
 		oob += eccbytes;
 
-		if (chip->ecc.postpad) {
-			chip->write_buf(mtd, oob, chip->ecc.postpad);
-			oob += chip->ecc.postpad;
+		if (nand_ecc(chip)->postpad) {
+			chip->write_buf(mtd, oob, nand_ecc(chip)->postpad);
+			oob += nand_ecc(chip)->postpad;
 		}
 	}
 
@@ -2269,7 +2279,7 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	int status, subpage;
 
 	if (!(chip->options & NAND_NO_SUBPAGE_WRITE) &&
-		chip->ecc.write_subpage)
+		nand_ecc(chip)->write_subpage)
 		subpage = offset || (data_len < mtd->writesize);
 	else
 		subpage = 0;
@@ -2277,13 +2287,15 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
-		status = chip->ecc.write_page_raw(mtd, chip, buf,
+		status = nand_ecc(chip)->write_page_raw(mtd, chip, buf,
 							oob_required);
 	else if (subpage)
-		status = chip->ecc.write_subpage(mtd, chip, offset, data_len,
-							 buf, oob_required);
+		status = nand_ecc(chip)->write_subpage(mtd, chip, offset,
+						       data_len, buf,
+						       oob_required);
 	else
-		status = chip->ecc.write_page(mtd, chip, buf, oob_required);
+		status = nand_ecc(chip)->write_page(mtd, chip, buf,
+						    oob_required);
 
 	if (status < 0)
 		return status;
@@ -2342,7 +2354,7 @@ static uint8_t *nand_fill_oob(struct mtd_info *mtd, uint8_t *oob, size_t len,
 		return oob + len;
 
 	case MTD_OPS_AUTO_OOB: {
-		struct nand_oobfree *free = chip->ecc.layout->oobfree;
+		struct nand_oobfree *free = nand_ecc(chip)->layout->oobfree;
 		uint32_t boffs = 0, woffs = ops->ooboffs;
 		size_t bytes = 0;
 
@@ -2582,7 +2594,7 @@ static int nand_do_write_oob(struct mtd_info *mtd, loff_t to,
 			 __func__, (unsigned int)to, (int)ops->ooblen);
 
 	if (ops->mode == MTD_OPS_AUTO_OOB)
-		len = chip->ecc.layout->oobavail;
+		len = nand_ecc(chip)->layout->oobavail;
 	else
 		len = mtd->oobsize;
 
@@ -2636,9 +2648,11 @@ static int nand_do_write_oob(struct mtd_info *mtd, loff_t to,
 	nand_fill_oob(mtd, ops->oobbuf, ops->ooblen, ops);
 
 	if (ops->mode == MTD_OPS_RAW)
-		status = chip->ecc.write_oob_raw(mtd, chip, page & chip->pagemask);
+		status = nand_ecc(chip)->write_oob_raw(mtd, chip,
+						       page & chip->pagemask);
 	else
-		status = chip->ecc.write_oob(mtd, chip, page & chip->pagemask);
+		status = nand_ecc(chip)->write_oob(mtd, chip,
+						   page & chip->pagemask);
 
 	chip->select_chip(mtd, -1);
 
@@ -3816,13 +3830,13 @@ static int nand_dt_init(struct mtd_info *mtd, struct nand_chip *chip,
 	}
 
 	if (ecc_mode >= 0)
-		chip->ecc.mode = ecc_mode;
+		nand_ecc(chip)->mode = ecc_mode;
 
 	if (ecc_strength >= 0)
-		chip->ecc.strength = ecc_strength;
+		nand_ecc(chip)->strength = ecc_strength;
 
 	if (ecc_step > 0)
-		chip->ecc.size = ecc_step;
+		nand_ecc(chip)->size = ecc_step;
 
 	return 0;
 }
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 272f429..2a9b557 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -1030,4 +1030,9 @@ struct nand_sdr_timings {
 
 /* get timing characteristics from ONFI timing mode. */
 const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode);
+
+static inline struct nand_ecc_ctrl *nand_ecc(struct nand_chip *chip)
+{
+	return &chip->ecc;
+}
 #endif /* __LINUX_MTD_NAND_H */
-- 
1.9.1

[RFC PATCH 5/6] mtd: nand: add infrastructure for per-partition ECC config

[Boris Brezillon]

Some SoCs require a different ECC config for their first stage bootloader,
but we cannot apply the same config on the whole flash, or we might have
to live an unsuitable ECC config (often weaker than what is required by
the NAND chip).

This patch makes use of the mtd_part_ops infrastructure to let the NAND
controller adjust the ECC config for each partition.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/nand_base.c | 152 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/nand.h     |  35 +++++++++-
 2 files changed, 186 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 67a29f5..9b027c6 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -4152,6 +4152,155 @@ static void nand_ecc_cleanup(struct mtd_info *mtd, struct nand_ecc_ctrl *ecc)
 		nand_bch_free((struct nand_bch_control *)ecc->priv);
 }
 
+static void nand_part_select(struct mtd_part *part)
+{
+	struct nand_chip *chip = part->master->priv;
+
+	if (chip->part_ops->select)
+		chip->part_ops->select(part);
+
+	chip->cur_part = part;
+}
+
+static void nand_part_deselect(struct mtd_info *mtd)
+{
+	struct nand_chip *chip = mtd->priv;
+
+	if (chip->part_ops->deselect)
+		chip->part_ops->deselect(chip->cur_part);
+
+	chip->cur_part = NULL;
+}
+
+static int nand_part_read(struct mtd_info *mtd, loff_t from, size_t len,
+			  size_t *retlen, u_char *buf)
+{
+	struct mtd_part *part = mtd_to_part(mtd);
+	struct mtd_ecc_stats stats;
+	struct mtd_oob_ops ops;
+	int ret;
+
+	nand_get_device(part->master, FL_READING);
+	stats = part->master->ecc_stats;
+
+	nand_part_select(part);
+	memset(&ops, 0, sizeof(ops));
+	ops.len = len;
+	ops.datbuf = buf;
+	ops.mode = MTD_OPS_PLACE_OOB;
+	ret = nand_do_read_ops(part->master, from + part->offset, &ops);
+	*retlen = ops.retlen;
+	nand_part_deselect(part->master);
+
+	if (unlikely(mtd_is_eccerr(ret)))
+		part->mtd.ecc_stats.failed +=
+			part->master->ecc_stats.failed - stats.failed;
+	else
+		part->mtd.ecc_stats.corrected +=
+			part->master->ecc_stats.corrected - stats.corrected;
+	nand_release_device(part->master);
+
+	return ret;
+}
+
+static int nand_part_write(struct mtd_info *mtd, loff_t to, size_t len,
+			   size_t *retlen, const u_char *buf)
+{
+	struct mtd_part *part = mtd_to_part(mtd);
+	struct mtd_oob_ops ops;
+	int ret;
+
+	nand_get_device(part->master, FL_WRITING);
+	nand_part_select(part);
+	memset(&ops, 0, sizeof(ops));
+	ops.len = len;
+	ops.datbuf = (uint8_t *)buf;
+	ops.mode = MTD_OPS_PLACE_OOB;
+	ret = nand_do_write_ops(part->master, to + part->offset, &ops);
+	*retlen = ops.retlen;
+	nand_part_deselect(part->master);
+	nand_release_device(part->master);
+	return ret;
+}
+
+static int panic_nand_part_write(struct mtd_info *mtd, loff_t to, size_t len,
+				 size_t *retlen, const uint8_t *buf)
+{
+	struct mtd_part *part = mtd_to_part(mtd);
+	struct nand_chip *chip = part->master->priv;
+	struct mtd_oob_ops ops;
+	int ret;
+
+	/* Wait for the device to get ready */
+	panic_nand_wait(part->master, chip, 400);
+
+	/* Grab the device */
+	panic_nand_get_device(chip, part->master, FL_WRITING);
+
+	memset(&ops, 0, sizeof(ops));
+	ops.len = len;
+	ops.datbuf = (uint8_t *)buf;
+	ops.mode = MTD_OPS_PLACE_OOB;
+
+	ret = nand_do_write_ops(part->master, to + part->offset, &ops);
+
+	*retlen = ops.retlen;
+	return ret;
+}
+
+static int nand_part_add(struct mtd_part *part)
+{
+	struct mtd_info *mtd = &part->mtd;
+	struct nand_chip *chip = part->master->priv;
+	struct nand_part *npart;
+	int ret;
+
+	npart = kzalloc(sizeof(*npart), GFP_KERNEL);
+	if (!npart)
+		return -ENOMEM;
+
+	mtd_part_set_priv(part, npart);
+	mtd->_read = nand_part_read;
+	mtd->_write = nand_part_write;
+	mtd->_panic_write = panic_nand_part_write;
+
+	if (chip->part_ops->add) {
+		ret = chip->part_ops->add(part);
+		if (ret) {
+			kfree(npart);
+			return ret;
+		}
+	}
+
+	if (npart->ecc)
+		nand_ecc_ctrl_init(&part->mtd, npart->ecc);
+	else
+		npart->ecc = &chip->ecc;
+
+	return 0;
+}
+
+static void nand_part_remove(struct mtd_part *part)
+{
+	struct nand_chip *chip = part->master->priv;
+	struct nand_part *npart = mtd_part_get_priv(part);
+
+	if (npart->ecc == &chip->ecc)
+		npart->ecc = NULL;
+	else
+		nand_ecc_cleanup(&part->mtd, npart->ecc);
+
+	if (chip->part_ops->remove)
+		chip->part_ops->remove(part);
+
+	kfree(part->mtd.priv);
+}
+
+static const struct mtd_part_ops nand_mtd_part_ops = {
+	.add = nand_part_add,
+	.remove = nand_part_remove,
+};
+
 /**
  * nand_scan_tail - [NAND Interface] Scan for the NAND device
  * @mtd: MTD device structure
@@ -4189,6 +4338,9 @@ int nand_scan_tail(struct mtd_info *mtd)
 	/* Set the internal oob buffer location, just after the page data */
 	chip->oob_poi = chip->buffers->databuf + mtd->writesize;
 
+	if (chip->part_ops)
+		mtd->part_ops = &nand_mtd_part_ops;
+
 	if (!chip->write_page)
 		chip->write_page = nand_write_page;
 
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 2a9b557..71b491d 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -538,6 +538,28 @@ struct nand_buffers {
 	uint8_t *databuf;
 };
 
+struct nand_part {
+	struct nand_ecc_ctrl *ecc;
+	void *priv;
+};
+
+static inline void *nand_part_get_priv(struct nand_part *part)
+{
+	return part->priv;
+}
+
+static inline void nand_part_set_priv(struct nand_part *part, void *priv)
+{
+	part->priv = priv;
+}
+
+struct nand_part_ops {
+	int (*add)(struct mtd_part *part);
+	void (*remove)(struct mtd_part *part);
+	void (*select)(struct mtd_part *part);
+	void (*deselect)(struct mtd_part *part);
+};
+
 /**
  * struct nand_chip - NAND Private Flash Chip Data
  * @IO_ADDR_R:		[BOARDSPECIFIC] address to read the 8 I/O lines of the
@@ -676,6 +698,7 @@ struct nand_chip {
 	int (*onfi_get_features)(struct mtd_info *mtd, struct nand_chip *chip,
 			int feature_addr, uint8_t *subfeature_para);
 	int (*setup_read_retry)(struct mtd_info *mtd, int retry_mode);
+	int (*select_part)(struct mtd_info *master, struct mtd_info *slave);
 
 	int chip_delay;
 	unsigned int options;
@@ -723,6 +746,9 @@ struct nand_chip {
 	struct nand_bbt_descr *badblock_pattern;
 
 	void *priv;
+
+	const struct nand_part_ops *part_ops;
+	struct mtd_part *cur_part;
 };
 
 /*
@@ -1033,6 +1059,13 @@ const struct nand_sdr_timings *onfi_async_timing_mode_to_sdr_timings(int mode);
 
 static inline struct nand_ecc_ctrl *nand_ecc(struct nand_chip *chip)
 {
-	return &chip->ecc;
+	struct nand_part *npart;
+
+	if (!chip->cur_part)
+		return &chip->ecc;
+
+	npart = chip->cur_part->mtd.priv;
+
+	return npart->ecc;
 }
 #endif /* __LINUX_MTD_NAND_H */
-- 
1.9.1

[RFC PATCH 6/6] mtd: nand: sunxi: support per-partition ECC config

[Boris Brezillon]

Allwinner SoCs have to support per-partition ECC in order to properly
expose the first stage bootloader partition which requires specific ECC
config that might not match the NAND chip requirements.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/mtd/nand/sunxi_nand.c | 61 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 57 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c
index 96dda70..6ced4c9 100644
--- a/drivers/mtd/nand/sunxi_nand.c
+++ b/drivers/mtd/nand/sunxi_nand.c
@@ -210,6 +210,10 @@ struct sunxi_nand_hw_ecc {
 	struct nand_ecclayout layout;
 };
 
+struct sunxi_nand_part {
+	struct nand_ecc_ctrl ecc;
+};
+
 /*
  * NAND chip structure: stores NAND chip device related information
  *
@@ -535,7 +539,7 @@ static int sunxi_nfc_hw_ecc_read_page(struct mtd_info *mtd,
 				      int oob_required, int page)
 {
 	struct sunxi_nfc *nfc = to_sunxi_nfc(chip->controller);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct nand_ecc_ctrl *ecc = nand_ecc(chip);
 	struct nand_ecclayout *layout = ecc->layout;
 	struct sunxi_nand_hw_ecc *data = ecc->priv;
 	unsigned int max_bitflips = 0;
@@ -621,7 +625,7 @@ static int sunxi_nfc_hw_ecc_write_page(struct mtd_info *mtd,
 				       const uint8_t *buf, int oob_required)
 {
 	struct sunxi_nfc *nfc = to_sunxi_nfc(chip->controller);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct nand_ecc_ctrl *ecc = nand_ecc(chip);
 	struct nand_ecclayout *layout = ecc->layout;
 	struct sunxi_nand_hw_ecc *data = ecc->priv;
 	int offset;
@@ -697,7 +701,7 @@ static int sunxi_nfc_hw_syndrome_ecc_read_page(struct mtd_info *mtd,
 					       int page)
 {
 	struct sunxi_nfc *nfc = to_sunxi_nfc(chip->controller);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct nand_ecc_ctrl *ecc = nand_ecc(chip);
 	struct sunxi_nand_hw_ecc *data = ecc->priv;
 	unsigned int max_bitflips = 0;
 	uint8_t *oob = chip->oob_poi;
@@ -765,7 +769,7 @@ static int sunxi_nfc_hw_syndrome_ecc_write_page(struct mtd_info *mtd,
 						int oob_required)
 {
 	struct sunxi_nfc *nfc = to_sunxi_nfc(chip->controller);
-	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct nand_ecc_ctrl *ecc = nand_ecc(chip);
 	struct sunxi_nand_hw_ecc *data = ecc->priv;
 	uint8_t *oob = chip->oob_poi;
 	int offset = 0;
@@ -1214,6 +1218,54 @@ static int sunxi_nand_ecc_init(struct mtd_info *mtd, struct nand_ecc_ctrl *ecc,
 	return 0;
 }
 
+static int sunxi_nand_part_add(struct mtd_part *part)
+{
+	struct nand_part *npart = mtd_part_get_priv(part);
+	struct device_node *np = part->mtd.dev.of_node;
+	struct sunxi_nand_part *spart;
+	int ret;
+
+	if (!np)
+		return 0;
+
+	spart = kzalloc(sizeof(*spart), GFP_KERNEL);
+	if (!spart)
+		return -ENOMEM;
+
+	/*
+	 * Only initialize partition specific ECC config if the nand-ecc-mode
+	 * property is provided.
+	 */
+	if (of_get_nand_ecc_mode(np) >= 0) {
+		ret = sunxi_nand_ecc_init(&part->mtd, &spart->ecc, np);
+		if (ret) {
+			kfree(spart);
+			return ret;
+		}
+
+		npart->ecc = &spart->ecc;
+	}
+
+	nand_part_set_priv(npart, spart);
+
+	return 0;
+}
+
+static void sunxi_nand_part_remove(struct mtd_part *part)
+{
+	struct nand_part *npart = mtd_part_get_priv(part);
+	struct sunxi_nand_part *spart = nand_part_get_priv(npart);
+
+	npart->ecc = NULL;
+	sunxi_nand_ecc_cleanup(&spart->ecc);
+	kfree(spart);
+}
+
+static const struct nand_part_ops sunxi_nand_part_ops = {
+	.add = sunxi_nand_part_add,
+	.remove = sunxi_nand_part_remove,
+};
+
 static int sunxi_nand_chip_init(struct device *dev, struct sunxi_nfc *nfc,
 				struct device_node *np)
 {
@@ -1317,6 +1369,7 @@ static int sunxi_nand_chip_init(struct device *dev, struct sunxi_nfc *nfc,
 	nand->read_buf = sunxi_nfc_read_buf;
 	nand->write_buf = sunxi_nfc_write_buf;
 	nand->read_byte = sunxi_nfc_read_byte;
+	nand->part_ops = &sunxi_nand_part_ops;
 
 	if (of_get_nand_on_flash_bbt(np))
 		nand->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
-- 
1.9.1

[linux-sunxi] [RFC PATCH 0/6] mtd: nand: per-partition ECC config

[Hans de Goede]

Hi all,

On 30-07-15 15:50, Boris Brezillon wrote:
> Hello,
>
> It's been a year and a half since I posted my first series proposing
> an approach to support per-partition ECC config [1].
>
> First of all, before describing what's done in this patch series, I'd
> like to sum-up why this is needed, and why a generic approach is
> preferred over a NAND controller specific one.
> On one side we have a lot of NAND chips out there and they all have
> their own requirements in term of ECC strength and step size. On the
> other side, most SoCs support booting from NAND (they embed a simple
> logic in the ROM code to access a NAND chip through their NAND
> controller).
> In a ideal world all NAND chips would use the ONFI or JEDEC standard
> exposing their requirements in a standard way, and the SoC vendors
> would put the ONFI and JEDEC detection code in their ROM code and use
> it to properly configure their NAND controller.
> But we're not leaving in an ideal world, and some SoC vendors have
> decided to hardcode (or use a simplified logic) to select the ECC
> controller config. And in the case where the NAND requirement does
> not match the ROM code config, you only have two solutions:
>
> 1/ leave with the unsuitable ECC config for the whole chip
> 2/ isolate the portion of NAND read by the ROM code into a sperate
>     partition and use a suitable ECC config for the rest of the
>     NAND
>
> IMHO the second solution is far better than the first one, but it
> requires some adjustments in the mtdpart and NAND code layer to be
> applicable.
>
> Now, why should we prefer a generic approach over a NAND controller/SoC
> specific one ?
> Because, this seems to be a problem faced by other people on other
> platform than the sunxi one. Moreover, the ECC config is not the only
> thing we'd have to tweak per partition: I'm currently working on the
> NAND randomizer/scrambler aspect (required to support some MLC chips),
> and this is also something the ROM code configure differently to
> boot the first stage bootloader.
> For all these reasons, I think providing a generic infrastructure allowing
> specific implementation to tweak their behavior is better than hardcoding
> it somewhere in the NAND controller driver.
>
> This series proposes a solution to allow such per-partition config by
> first letting MTD implementations (or subframework) overload the MTD
> partition functions (patches 1 and 2), and then providing the appropriate
> modifications in the NAND layer to support per-partition ECC config
> (patches 3 to 5).
> The last patch is showing how a NAND controller can add support for
> per-partition ECC config.
>
> Note that I tried to keep the changes as less invasive as impossible, but
> I might have missed some aspects.
> Any suggestions are welcome.
>
> Best Regards,
>
> Boris

Thanks for doing this, this series looks good to me.

MTD maintainers, what do we need to do to get this upstream, submit
a non RFC version ? Or ... ?

Regards,

Hans