mxc_nand controller fixes

mxc_nand controller fixes

[Sascha Hauer]

This series fixes several problems in the mxc_nand driver.

- Raw support does not work since hardware ECC is unconditionally
  enabled
- The v2/v3 controller code returns the number of corrected
  bitflips for the whole page for each subpage leading to 
  results for times too high
- The v1 controller code returns the number of corrected bitflips
  only for the last subpage. On 2k page NANDs bitflips in the
  first three subpages remain uncovered

This series fixes these issues. Tested with the mtd test modules
mtd_nandbiterrs, mtd_oobtest, mtd_pagetest and mtd_readtest on
a i.MX27 board which is a v1 controller and a i.MX25 board which
is a v2 controller. Boris, and yes, I just tried some of the userspace
mtd tests aswell, namely nandtest, nandsubpagetest and nandbiterrs
;)

Based on nand/next.

Sascha

----------------------------------------------------------------
Sascha Hauer (8):
      mtd: nand: mxc: reorder functions to avoid forward declarations
      mtd: nand: mxc: Add function to control hardware ECC
      mtd: nand: mxc: Add buffer argument to copy_spare
      mtd: nand: mxc: Fix failed/corrected values for v2/v3 controllers
      mtd: nand: mxc: Fix failed/corrected values for v1 controllers
      mtd: nand: mxc: Add own write_page
      mtd: nand: mxc: Drop now unnecessary functions
      mtd: nand: mxc: remove now unused code

 drivers/mtd/nand/mxc_nand.c | 519 +++++++++++++++++++++++++++-----------------
 1 file changed, 322 insertions(+), 197 deletions(-)

[PATCH 1/8] mtd: nand: mxc: reorder functions to avoid forward declarations

[Sascha Hauer]

We'll call copy_spare() and mxc_do_addr_cycle() from another place
during the next patches, so move functions up to avoid forward
declarations.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/mxc_nand.c | 206 ++++++++++++++++++++++----------------------
 1 file changed, 103 insertions(+), 103 deletions(-)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index f3be0b2a8869..6741af990dc4 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -252,6 +252,109 @@ static void memcpy16_toio(void __iomem *trg, const void *src, int size)
 		__raw_writew(*s++, t++);
 }
 
+/*
+ * The controller splits a page into data chunks of 512 bytes + partial oob.
+ * There are writesize / 512 such chunks, the size of the partial oob parts is
+ * oobsize / #chunks rounded down to a multiple of 2. The last oob chunk then
+ * contains additionally the byte lost by rounding (if any).
+ * This function handles the needed shuffling between host->data_buf (which
+ * holds a page in natural order, i.e. writesize bytes data + oobsize bytes
+ * spare) and the NFC buffer.
+ */
+static void copy_spare(struct mtd_info *mtd, bool bfrom)
+{
+	struct nand_chip *this = mtd_to_nand(mtd);
+	struct mxc_nand_host *host = nand_get_controller_data(this);
+	u16 i, oob_chunk_size;
+	u16 num_chunks = mtd->writesize / 512;
+
+	u8 *d = host->data_buf + mtd->writesize;
+	u8 __iomem *s = host->spare0;
+	u16 sparebuf_size = host->devtype_data->spare_len;
+
+	/* size of oob chunk for all but possibly the last one */
+	oob_chunk_size = (host->used_oobsize / num_chunks) & ~1;
+
+	if (bfrom) {
+		for (i = 0; i < num_chunks - 1; i++)
+			memcpy16_fromio(d + i * oob_chunk_size,
+					s + i * sparebuf_size,
+					oob_chunk_size);
+
+		/* the last chunk */
+		memcpy16_fromio(d + i * oob_chunk_size,
+				s + i * sparebuf_size,
+				host->used_oobsize - i * oob_chunk_size);
+	} else {
+		for (i = 0; i < num_chunks - 1; i++)
+			memcpy16_toio(&s[i * sparebuf_size],
+				      &d[i * oob_chunk_size],
+				      oob_chunk_size);
+
+		/* the last chunk */
+		memcpy16_toio(&s[i * sparebuf_size],
+			      &d[i * oob_chunk_size],
+			      host->used_oobsize - i * oob_chunk_size);
+	}
+}
+
+/*
+ * MXC NANDFC can only perform full page+spare or spare-only read/write.  When
+ * the upper layers perform a read/write buf operation, the saved column address
+ * is used to index into the full page. So usually this function is called with
+ * column == 0 (unless no column cycle is needed indicated by column == -1)
+ */
+static void mxc_do_addr_cycle(struct mtd_info *mtd, int column, int page_addr)
+{
+	struct nand_chip *nand_chip = mtd_to_nand(mtd);
+	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
+
+	/* Write out column address, if necessary */
+	if (column != -1) {
+		host->devtype_data->send_addr(host, column & 0xff,
+					      page_addr == -1);
+		if (mtd->writesize > 512)
+			/* another col addr cycle for 2k page */
+			host->devtype_data->send_addr(host,
+						      (column >> 8) & 0xff,
+						      false);
+	}
+
+	/* Write out page address, if necessary */
+	if (page_addr != -1) {
+		/* paddr_0 - p_addr_7 */
+		host->devtype_data->send_addr(host, (page_addr & 0xff), false);
+
+		if (mtd->writesize > 512) {
+			if (mtd->size >= 0x10000000) {
+				/* paddr_8 - paddr_15 */
+				host->devtype_data->send_addr(host,
+						(page_addr >> 8) & 0xff,
+						false);
+				host->devtype_data->send_addr(host,
+						(page_addr >> 16) & 0xff,
+						true);
+			} else
+				/* paddr_8 - paddr_15 */
+				host->devtype_data->send_addr(host,
+						(page_addr >> 8) & 0xff, true);
+		} else {
+			if (nand_chip->options & NAND_ROW_ADDR_3) {
+				/* paddr_8 - paddr_15 */
+				host->devtype_data->send_addr(host,
+						(page_addr >> 8) & 0xff,
+						false);
+				host->devtype_data->send_addr(host,
+						(page_addr >> 16) & 0xff,
+						true);
+			} else
+				/* paddr_8 - paddr_15 */
+				host->devtype_data->send_addr(host,
+						(page_addr >> 8) & 0xff, true);
+		}
+	}
+}
+
 static int check_int_v3(struct mxc_nand_host *host)
 {
 	uint32_t tmp;
@@ -772,109 +875,6 @@ static void mxc_nand_select_chip_v2(struct mtd_info *mtd, int chip)
 	writew(host->active_cs << 4, NFC_V1_V2_BUF_ADDR);
 }
 
-/*
- * The controller splits a page into data chunks of 512 bytes + partial oob.
- * There are writesize / 512 such chunks, the size of the partial oob parts is
- * oobsize / #chunks rounded down to a multiple of 2. The last oob chunk then
- * contains additionally the byte lost by rounding (if any).
- * This function handles the needed shuffling between host->data_buf (which
- * holds a page in natural order, i.e. writesize bytes data + oobsize bytes
- * spare) and the NFC buffer.
- */
-static void copy_spare(struct mtd_info *mtd, bool bfrom)
-{
-	struct nand_chip *this = mtd_to_nand(mtd);
-	struct mxc_nand_host *host = nand_get_controller_data(this);
-	u16 i, oob_chunk_size;
-	u16 num_chunks = mtd->writesize / 512;
-
-	u8 *d = host->data_buf + mtd->writesize;
-	u8 __iomem *s = host->spare0;
-	u16 sparebuf_size = host->devtype_data->spare_len;
-
-	/* size of oob chunk for all but possibly the last one */
-	oob_chunk_size = (host->used_oobsize / num_chunks) & ~1;
-
-	if (bfrom) {
-		for (i = 0; i < num_chunks - 1; i++)
-			memcpy16_fromio(d + i * oob_chunk_size,
-					s + i * sparebuf_size,
-					oob_chunk_size);
-
-		/* the last chunk */
-		memcpy16_fromio(d + i * oob_chunk_size,
-				s + i * sparebuf_size,
-				host->used_oobsize - i * oob_chunk_size);
-	} else {
-		for (i = 0; i < num_chunks - 1; i++)
-			memcpy16_toio(&s[i * sparebuf_size],
-				      &d[i * oob_chunk_size],
-				      oob_chunk_size);
-
-		/* the last chunk */
-		memcpy16_toio(&s[i * sparebuf_size],
-			      &d[i * oob_chunk_size],
-			      host->used_oobsize - i * oob_chunk_size);
-	}
-}
-
-/*
- * MXC NANDFC can only perform full page+spare or spare-only read/write.  When
- * the upper layers perform a read/write buf operation, the saved column address
- * is used to index into the full page. So usually this function is called with
- * column == 0 (unless no column cycle is needed indicated by column == -1)
- */
-static void mxc_do_addr_cycle(struct mtd_info *mtd, int column, int page_addr)
-{
-	struct nand_chip *nand_chip = mtd_to_nand(mtd);
-	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
-
-	/* Write out column address, if necessary */
-	if (column != -1) {
-		host->devtype_data->send_addr(host, column & 0xff,
-					      page_addr == -1);
-		if (mtd->writesize > 512)
-			/* another col addr cycle for 2k page */
-			host->devtype_data->send_addr(host,
-						      (column >> 8) & 0xff,
-						      false);
-	}
-
-	/* Write out page address, if necessary */
-	if (page_addr != -1) {
-		/* paddr_0 - p_addr_7 */
-		host->devtype_data->send_addr(host, (page_addr & 0xff), false);
-
-		if (mtd->writesize > 512) {
-			if (mtd->size >= 0x10000000) {
-				/* paddr_8 - paddr_15 */
-				host->devtype_data->send_addr(host,
-						(page_addr >> 8) & 0xff,
-						false);
-				host->devtype_data->send_addr(host,
-						(page_addr >> 16) & 0xff,
-						true);
-			} else
-				/* paddr_8 - paddr_15 */
-				host->devtype_data->send_addr(host,
-						(page_addr >> 8) & 0xff, true);
-		} else {
-			if (nand_chip->options & NAND_ROW_ADDR_3) {
-				/* paddr_8 - paddr_15 */
-				host->devtype_data->send_addr(host,
-						(page_addr >> 8) & 0xff,
-						false);
-				host->devtype_data->send_addr(host,
-						(page_addr >> 16) & 0xff,
-						true);
-			} else
-				/* paddr_8 - paddr_15 */
-				host->devtype_data->send_addr(host,
-						(page_addr >> 8) & 0xff, true);
-		}
-	}
-}
-
 #define MXC_V1_ECCBYTES		5
 
 static int mxc_v1_ooblayout_ecc(struct mtd_info *mtd, int section,
-- 
2.11.0

[PATCH 2/8] mtd: nand: mxc: Add function to control hardware ECC

[Sascha Hauer]

For proper raw read/write support need to be able to control the
hardware ECC engine. Add a function to enable/disable it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/mxc_nand.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 6741af990dc4..019c58579dbc 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -154,6 +154,7 @@ struct mxc_nand_devtype_data {
 			u_char *read_ecc, u_char *calc_ecc);
 	int (*setup_data_interface)(struct mtd_info *mtd, int csline,
 				    const struct nand_data_interface *conf);
+	void (*enable_hwecc)(struct nand_chip *chip, bool enable);
 
 	/*
 	 * On i.MX21 the CONFIG2:INT bit cannot be read if interrupts are masked
@@ -678,6 +679,42 @@ static uint16_t get_dev_status_v1_v2(struct mxc_nand_host *host)
 	return ret;
 }
 
+static void mxc_nand_enable_hwecc_v1_v2(struct nand_chip *chip, bool enable)
+{
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+	uint16_t config1;
+
+	if (chip->ecc.mode != NAND_ECC_HW)
+		return;
+
+	config1 = readw(NFC_V1_V2_CONFIG1);
+
+	if (enable)
+		config1 |= NFC_V1_V2_CONFIG1_ECC_EN;
+	else
+		config1 &= ~NFC_V1_V2_CONFIG1_ECC_EN;
+
+	writew(config1, NFC_V1_V2_CONFIG1);
+}
+
+static void mxc_nand_enable_hwecc_v3(struct nand_chip *chip, bool enable)
+{
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+	uint32_t config2;
+
+	if (chip->ecc.mode != NAND_ECC_HW)
+		return;
+
+	config2 = readl(NFC_V3_CONFIG2);
+
+	if (enable)
+		config2 |= NFC_V3_CONFIG2_ECC_EN;
+	else
+		config2 &= ~NFC_V3_CONFIG2_ECC_EN;
+
+	writel(config2, NFC_V3_CONFIG2);
+}
+
 /* This functions is used by upper layer to checks if device is ready */
 static int mxc_nand_dev_ready(struct mtd_info *mtd)
 {
@@ -1408,6 +1445,7 @@ static const struct mxc_nand_devtype_data imx21_nand_devtype_data = {
 	.ooblayout = &mxc_v1_ooblayout_ops,
 	.select_chip = mxc_nand_select_chip_v1_v3,
 	.correct_data = mxc_nand_correct_data_v1,
+	.enable_hwecc = mxc_nand_enable_hwecc_v1_v2,
 	.irqpending_quirk = 1,
 	.needs_ip = 0,
 	.regs_offset = 0xe00,
@@ -1431,6 +1469,7 @@ static const struct mxc_nand_devtype_data imx27_nand_devtype_data = {
 	.ooblayout = &mxc_v1_ooblayout_ops,
 	.select_chip = mxc_nand_select_chip_v1_v3,
 	.correct_data = mxc_nand_correct_data_v1,
+	.enable_hwecc = mxc_nand_enable_hwecc_v1_v2,
 	.irqpending_quirk = 0,
 	.needs_ip = 0,
 	.regs_offset = 0xe00,
@@ -1456,6 +1495,7 @@ static const struct mxc_nand_devtype_data imx25_nand_devtype_data = {
 	.select_chip = mxc_nand_select_chip_v2,
 	.correct_data = mxc_nand_correct_data_v2_v3,
 	.setup_data_interface = mxc_nand_v2_setup_data_interface,
+	.enable_hwecc = mxc_nand_enable_hwecc_v1_v2,
 	.irqpending_quirk = 0,
 	.needs_ip = 0,
 	.regs_offset = 0x1e00,
@@ -1480,6 +1520,7 @@ static const struct mxc_nand_devtype_data imx51_nand_devtype_data = {
 	.ooblayout = &mxc_v2_ooblayout_ops,
 	.select_chip = mxc_nand_select_chip_v1_v3,
 	.correct_data = mxc_nand_correct_data_v2_v3,
+	.enable_hwecc = mxc_nand_enable_hwecc_v3,
 	.irqpending_quirk = 0,
 	.needs_ip = 1,
 	.regs_offset = 0,
@@ -1505,6 +1546,7 @@ static const struct mxc_nand_devtype_data imx53_nand_devtype_data = {
 	.ooblayout = &mxc_v2_ooblayout_ops,
 	.select_chip = mxc_nand_select_chip_v1_v3,
 	.correct_data = mxc_nand_correct_data_v2_v3,
+	.enable_hwecc = mxc_nand_enable_hwecc_v3,
 	.irqpending_quirk = 0,
 	.needs_ip = 1,
 	.regs_offset = 0,
-- 
2.11.0

[PATCH 3/8] mtd: nand: mxc: Add buffer argument to copy_spare

[Sascha Hauer]

With following patches we will have to copy the spare data to/from
other buffers, so add the buffer as argument to copy_spare().

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/mxc_nand.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 019c58579dbc..ab9cd45237d3 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -262,14 +262,14 @@ static void memcpy16_toio(void __iomem *trg, const void *src, int size)
  * holds a page in natural order, i.e. writesize bytes data + oobsize bytes
  * spare) and the NFC buffer.
  */
-static void copy_spare(struct mtd_info *mtd, bool bfrom)
+static void copy_spare(struct mtd_info *mtd, bool bfrom, void *buf)
 {
 	struct nand_chip *this = mtd_to_nand(mtd);
 	struct mxc_nand_host *host = nand_get_controller_data(this);
 	u16 i, oob_chunk_size;
 	u16 num_chunks = mtd->writesize / 512;
 
-	u8 *d = host->data_buf + mtd->writesize;
+	u8 *d = buf;
 	u8 __iomem *s = host->spare0;
 	u16 sparebuf_size = host->devtype_data->spare_len;
 
@@ -1295,7 +1295,7 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
 
 		memcpy32_fromio(host->data_buf, host->main_area0,
 				mtd->writesize);
-		copy_spare(mtd, true);
+		copy_spare(mtd, true, host->data_buf + mtd->writesize);
 		break;
 
 	case NAND_CMD_SEQIN:
@@ -1314,7 +1314,7 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
 
 	case NAND_CMD_PAGEPROG:
 		memcpy32_toio(host->main_area0, host->data_buf, mtd->writesize);
-		copy_spare(mtd, false);
+		copy_spare(mtd, false, host->data_buf + mtd->writesize);
 		host->devtype_data->send_page(mtd, NFC_INPUT);
 		host->devtype_data->send_cmd(host, command, true);
 		WARN_ONCE(column != -1 || page_addr != -1,
-- 
2.11.0

[PATCH 4/8] mtd: nand: mxc: Fix failed/corrected values for v2/v3 controllers

[Sascha Hauer]

Currently nand_read_page_hwecc() from nand_base calls
mxc_nand_correct_data_v2_v3() for each subpage, but in this function we
return the corrected/failed results for the whole page instead
of a single subpage. On a 2k page size Nand this leads to results which
are 4 times too high.
The whole ecc.calculate/ecc.correct mechanism used by
nand_read_page_hwecc() is not suitable for devices which correct the
data in hardware, so fix this by using a driver specific read_page
function which does the right thing. Also add read_page_raw and read_oob
For proper raw and oob read support.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/mxc_nand.c | 84 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 76 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index ab9cd45237d3..d698f7c6666c 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -140,6 +140,8 @@ struct mxc_nand_host;
 
 struct mxc_nand_devtype_data {
 	void (*preset)(struct mtd_info *);
+	int (*read_page)(struct mtd_info *mtd, struct nand_chip *chip,
+			void *buf, void *oob, bool ecc, int page);
 	void (*send_cmd)(struct mxc_nand_host *, uint16_t, int);
 	void (*send_addr)(struct mxc_nand_host *, uint16_t, int);
 	void (*send_page)(struct mtd_info *, unsigned int);
@@ -757,13 +759,35 @@ static int mxc_nand_correct_data_v1(struct mtd_info *mtd, u_char *dat,
 static int mxc_nand_correct_data_v2_v3(struct mtd_info *mtd, u_char *dat,
 				 u_char *read_ecc, u_char *calc_ecc)
 {
+	return 0;
+}
+
+static int mxc_nand_read_page_v2_v3(struct mtd_info *mtd, struct nand_chip *chip,
+				    void *buf, void *oob, bool ecc, int page)
+{
 	struct nand_chip *nand_chip = mtd_to_nand(mtd);
-	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+	unsigned int max_bitflips = 0;
 	u32 ecc_stat, err;
-	int no_subpages = 1;
-	int ret = 0;
+	int no_subpages;
 	u8 ecc_bit_mask, err_limit;
 
+	host->devtype_data->enable_hwecc(nand_chip, ecc);
+
+	host->devtype_data->send_cmd(host, NAND_CMD_READ0, false);
+	mxc_do_addr_cycle(mtd, 0, page);
+
+	if (mtd->writesize > 512)
+		host->devtype_data->send_cmd(host,
+				NAND_CMD_READSTART, true);
+
+	host->devtype_data->send_page(mtd, NFC_OUTPUT);
+
+	if (buf)
+		memcpy32_fromio(buf, host->main_area0, mtd->writesize);
+	if (oob)
+		copy_spare(mtd, true, oob);
+
 	ecc_bit_mask = (host->eccsize == 4) ? 0x7 : 0xf;
 	err_limit = (host->eccsize == 4) ? 0x4 : 0x8;
 
@@ -774,17 +798,53 @@ static int mxc_nand_correct_data_v2_v3(struct mtd_info *mtd, u_char *dat,
 	do {
 		err = ecc_stat & ecc_bit_mask;
 		if (err > err_limit) {
-			dev_dbg(host->dev, "UnCorrectable RS-ECC Error\n");
-			return -EBADMSG;
+			mtd->ecc_stats.failed++;
 		} else {
-			ret += err;
+			mtd->ecc_stats.corrected += err;
+			max_bitflips = max_t(unsigned int, max_bitflips, err);
 		}
+
 		ecc_stat >>= 4;
 	} while (--no_subpages);
 
-	dev_dbg(host->dev, "%d Symbol Correctable RS-ECC Error\n", ret);
+	return max_bitflips;
+}
 
-	return ret;
+static int mxc_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
+			      uint8_t *buf, int oob_required, int page)
+{
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+	void *oob_buf;
+
+	if (oob_required)
+		oob_buf = chip->oob_poi;
+	else
+		oob_buf = NULL;
+
+	return host->devtype_data->read_page(mtd, chip, buf, oob_buf, 1, page);
+}
+
+static int mxc_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+				  uint8_t *buf, int oob_required, int page)
+{
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+	void *oob_buf;
+
+	if (oob_required)
+		oob_buf = chip->oob_poi;
+	else
+		oob_buf = NULL;
+
+	return host->devtype_data->read_page(mtd, chip, buf, oob_buf, 0, page);
+}
+
+static int mxc_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
+			     int page)
+{
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+
+	return host->devtype_data->read_page(mtd, chip, NULL, chip->oob_poi, 0,
+					     page);
 }
 
 static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
@@ -1483,6 +1543,7 @@ static const struct mxc_nand_devtype_data imx27_nand_devtype_data = {
 /* v21: i.MX25, i.MX35 */
 static const struct mxc_nand_devtype_data imx25_nand_devtype_data = {
 	.preset = preset_v2,
+	.read_page = mxc_nand_read_page_v2_v3,
 	.send_cmd = send_cmd_v1_v2,
 	.send_addr = send_addr_v1_v2,
 	.send_page = send_page_v2,
@@ -1509,6 +1570,7 @@ static const struct mxc_nand_devtype_data imx25_nand_devtype_data = {
 /* v3.2a: i.MX51 */
 static const struct mxc_nand_devtype_data imx51_nand_devtype_data = {
 	.preset = preset_v3,
+	.read_page = mxc_nand_read_page_v2_v3,
 	.send_cmd = send_cmd_v3,
 	.send_addr = send_addr_v3,
 	.send_page = send_page_v3,
@@ -1535,6 +1597,7 @@ static const struct mxc_nand_devtype_data imx51_nand_devtype_data = {
 /* v3.2b: i.MX53 */
 static const struct mxc_nand_devtype_data imx53_nand_devtype_data = {
 	.preset = preset_v3,
+	.read_page = mxc_nand_read_page_v2_v3,
 	.send_cmd = send_cmd_v3,
 	.send_addr = send_addr_v3,
 	.send_page = send_page_v3,
@@ -1793,6 +1856,11 @@ static int mxcnd_probe(struct platform_device *pdev)
 
 	switch (this->ecc.mode) {
 	case NAND_ECC_HW:
+		if (host->devtype_data->read_page) {
+			this->ecc.read_page = mxc_nand_read_page;
+			this->ecc.read_page_raw = mxc_nand_read_page_raw;
+			this->ecc.read_oob = mxc_nand_read_oob;
+		}
 		this->ecc.calculate = mxc_nand_calculate_ecc;
 		this->ecc.hwctl = mxc_nand_enable_hwecc;
 		this->ecc.correct = host->devtype_data->correct_data;
-- 
2.11.0

[PATCH 5/8] mtd: nand: mxc: Fix failed/corrected values for v1 controllers

[Sascha Hauer]

The v1 controller code has several flaws:
- We do not forward the number of corrected bitflips to the upper layers
- For 2k page NAND chips only the status results from the fourth subpage
  read are evaluated, so ECC failures in the other subpages remain
  uncovered
- When there are uncorrectable errors we have to increase the statistics
  counter, but still have to return successfully. Currently we return
  an error

This patch fixes this by introducing a v1 specific read_page function.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/mxc_nand.c | 78 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 62 insertions(+), 16 deletions(-)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index d698f7c6666c..83bb01ab7079 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -738,22 +738,68 @@ static void mxc_nand_enable_hwecc(struct mtd_info *mtd, int mode)
 static int mxc_nand_correct_data_v1(struct mtd_info *mtd, u_char *dat,
 				 u_char *read_ecc, u_char *calc_ecc)
 {
+	return 0;
+}
+
+static int mxc_nand_read_page_v1(struct mtd_info *mtd, struct nand_chip *chip,
+				 void *buf, void *oob, bool ecc, int page)
+{
 	struct nand_chip *nand_chip = mtd_to_nand(mtd);
-	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+	unsigned int max_bitflips = 0;
+	int no_subpages;
+	int i;
 
-	/*
-	 * 1-Bit errors are automatically corrected in HW.  No need for
-	 * additional correction.  2-Bit errors cannot be corrected by
-	 * HW ECC, so we need to return failure
-	 */
-	uint16_t ecc_status = get_ecc_status_v1(host);
+	host->devtype_data->enable_hwecc(nand_chip, ecc);
+
+	host->devtype_data->send_cmd(host, NAND_CMD_READ0, false);
+	mxc_do_addr_cycle(mtd, 0, page);
+
+	if (mtd->writesize > 512)
+		host->devtype_data->send_cmd(host, NAND_CMD_READSTART, true);
 
-	if (((ecc_status & 0x3) == 2) || ((ecc_status >> 2) == 2)) {
-		dev_dbg(host->dev, "HWECC uncorrectable 2-bit ECC error\n");
-		return -EBADMSG;
+	no_subpages = mtd->writesize >> 9;
+
+	for (i = 0; i < no_subpages; i++) {
+		int flips = 0;
+		uint16_t ecc_stats;
+
+		/* NANDFC buffer 0 is used for page read/write */
+		writew((host->active_cs << 4) | i, NFC_V1_V2_BUF_ADDR);
+
+		writew(NFC_OUTPUT, NFC_V1_V2_CONFIG2);
+
+		/* Wait for operation to complete */
+		wait_op_done(host, true);
+
+		ecc_stats = get_ecc_status_v1(host);
+
+		ecc_stats >>= 2;
+
+		if (buf && ecc) {
+			switch (ecc_stats & 0x3) {
+			case 0:
+			default:
+				break;
+			case 1:
+				mtd->ecc_stats.corrected++;
+				flips++;
+				break;
+			case 2:
+				mtd->ecc_stats.failed++;
+				break;
+			}
+		}
+
+		max_bitflips = max_t(unsigned int, max_bitflips, flips);
 	}
 
-	return 0;
+	if (buf)
+		memcpy32_fromio(buf, host->main_area0, mtd->writesize);
+	if (oob)
+		copy_spare(mtd, true, oob);
+
+	return max_bitflips;
 }
 
 static int mxc_nand_correct_data_v2_v3(struct mtd_info *mtd, u_char *dat,
@@ -1494,6 +1540,7 @@ static struct nand_bbt_descr bbt_mirror_descr = {
 /* v1 + irqpending_quirk: i.MX21 */
 static const struct mxc_nand_devtype_data imx21_nand_devtype_data = {
 	.preset = preset_v1,
+	.read_page = mxc_nand_read_page_v1,
 	.send_cmd = send_cmd_v1_v2,
 	.send_addr = send_addr_v1_v2,
 	.send_page = send_page_v1,
@@ -1518,6 +1565,7 @@ static const struct mxc_nand_devtype_data imx21_nand_devtype_data = {
 /* v1 + !irqpending_quirk: i.MX27, i.MX31 */
 static const struct mxc_nand_devtype_data imx27_nand_devtype_data = {
 	.preset = preset_v1,
+	.read_page = mxc_nand_read_page_v1,
 	.send_cmd = send_cmd_v1_v2,
 	.send_addr = send_addr_v1_v2,
 	.send_page = send_page_v1,
@@ -1856,11 +1904,9 @@ static int mxcnd_probe(struct platform_device *pdev)
 
 	switch (this->ecc.mode) {
 	case NAND_ECC_HW:
-		if (host->devtype_data->read_page) {
-			this->ecc.read_page = mxc_nand_read_page;
-			this->ecc.read_page_raw = mxc_nand_read_page_raw;
-			this->ecc.read_oob = mxc_nand_read_oob;
-		}
+		this->ecc.read_page = mxc_nand_read_page;
+		this->ecc.read_page_raw = mxc_nand_read_page_raw;
+		this->ecc.read_oob = mxc_nand_read_oob;
 		this->ecc.calculate = mxc_nand_calculate_ecc;
 		this->ecc.hwctl = mxc_nand_enable_hwecc;
 		this->ecc.correct = host->devtype_data->correct_data;
-- 
2.11.0

[PATCH 6/8] mtd: nand: mxc: Add own write_page

[Sascha Hauer]

Now that we have our own read_page function add a write_page function
for consistency aswell. This can be a lot easier than the generic
function since we do not have to iterate over subpages but can write
the whole page at once. Also add write_page_raw and write_oob for
proper raw and oob write support.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/mxc_nand.c | 56 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 83bb01ab7079..6fbf98851a96 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -893,6 +893,59 @@ static int mxc_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 					     page);
 }
 
+static int __mxc_nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
+				 const uint8_t *buf, int oob_required, int page)
+{
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+
+	host->devtype_data->send_cmd(host, NAND_CMD_SEQIN, false);
+	mxc_do_addr_cycle(mtd, 0, page);
+
+	memcpy32_toio(host->main_area0, buf, mtd->writesize);
+	copy_spare(mtd, false, chip->oob_poi);
+
+	host->devtype_data->send_page(mtd, NFC_INPUT);
+	host->devtype_data->send_cmd(host, NAND_CMD_PAGEPROG, true);
+	mxc_do_addr_cycle(mtd, 0, page);
+
+	return 0;
+}
+
+static int mxc_nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
+			       const uint8_t *buf, int oob_required, int page)
+{
+	struct nand_chip *nand_chip = mtd_to_nand(mtd);
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+
+	host->devtype_data->enable_hwecc(nand_chip, true);
+
+	return __mxc_nand_write_page(mtd, chip, buf, oob_required, page);
+}
+
+static int mxc_nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+				   const uint8_t *buf, int oob_required, int page)
+{
+	struct nand_chip *nand_chip = mtd_to_nand(mtd);
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+
+	host->devtype_data->enable_hwecc(nand_chip, false);
+
+	return __mxc_nand_write_page(mtd, chip, buf, oob_required, page);
+}
+
+static int mxc_nand_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
+			      int page)
+{
+	struct nand_chip *nand_chip = mtd_to_nand(mtd);
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+
+	memset(host->data_buf, 0xff, mtd->writesize);
+
+	host->devtype_data->enable_hwecc(nand_chip, false);
+
+	return __mxc_nand_write_page(mtd, chip, host->data_buf, 1, page);
+}
+
 static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
 				  u_char *ecc_code)
 {
@@ -1907,6 +1960,9 @@ static int mxcnd_probe(struct platform_device *pdev)
 		this->ecc.read_page = mxc_nand_read_page;
 		this->ecc.read_page_raw = mxc_nand_read_page_raw;
 		this->ecc.read_oob = mxc_nand_read_oob;
+		this->ecc.write_page = mxc_nand_write_page;
+		this->ecc.write_page_raw = mxc_nand_write_page_raw;
+		this->ecc.write_oob = mxc_nand_write_oob;
 		this->ecc.calculate = mxc_nand_calculate_ecc;
 		this->ecc.hwctl = mxc_nand_enable_hwecc;
 		this->ecc.correct = host->devtype_data->correct_data;
-- 
2.11.0

[PATCH 7/8] mtd: nand: mxc: Drop now unnecessary functions

[Sascha Hauer]

Since we have our own read_page/write_page functions correct_data and
calculate are no longer needed. Remove them.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/mxc_nand.c | 36 ------------------------------------
 1 file changed, 36 deletions(-)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 6fbf98851a96..1d2c55e5ca89 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -152,8 +152,6 @@ struct mxc_nand_devtype_data {
 	u32 (*get_ecc_status)(struct mxc_nand_host *);
 	const struct mtd_ooblayout_ops *ooblayout;
 	void (*select_chip)(struct mtd_info *mtd, int chip);
-	int (*correct_data)(struct mtd_info *mtd, u_char *dat,
-			u_char *read_ecc, u_char *calc_ecc);
 	int (*setup_data_interface)(struct mtd_info *mtd, int csline,
 				    const struct nand_data_interface *conf);
 	void (*enable_hwecc)(struct nand_chip *chip, bool enable);
@@ -727,20 +725,6 @@ static int mxc_nand_dev_ready(struct mtd_info *mtd)
 	return 1;
 }
 
-static void mxc_nand_enable_hwecc(struct mtd_info *mtd, int mode)
-{
-	/*
-	 * If HW ECC is enabled, we turn it on during init. There is
-	 * no need to enable again here.
-	 */
-}
-
-static int mxc_nand_correct_data_v1(struct mtd_info *mtd, u_char *dat,
-				 u_char *read_ecc, u_char *calc_ecc)
-{
-	return 0;
-}
-
 static int mxc_nand_read_page_v1(struct mtd_info *mtd, struct nand_chip *chip,
 				 void *buf, void *oob, bool ecc, int page)
 {
@@ -802,12 +786,6 @@ static int mxc_nand_read_page_v1(struct mtd_info *mtd, struct nand_chip *chip,
 	return max_bitflips;
 }
 
-static int mxc_nand_correct_data_v2_v3(struct mtd_info *mtd, u_char *dat,
-				 u_char *read_ecc, u_char *calc_ecc)
-{
-	return 0;
-}
-
 static int mxc_nand_read_page_v2_v3(struct mtd_info *mtd, struct nand_chip *chip,
 				    void *buf, void *oob, bool ecc, int page)
 {
@@ -946,12 +924,6 @@ static int mxc_nand_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
 	return __mxc_nand_write_page(mtd, chip, host->data_buf, 1, page);
 }
 
-static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
-				  u_char *ecc_code)
-{
-	return 0;
-}
-
 static u_char mxc_nand_read_byte(struct mtd_info *mtd)
 {
 	struct nand_chip *nand_chip = mtd_to_nand(mtd);
@@ -1604,7 +1576,6 @@ static const struct mxc_nand_devtype_data imx21_nand_devtype_data = {
 	.get_ecc_status = get_ecc_status_v1,
 	.ooblayout = &mxc_v1_ooblayout_ops,
 	.select_chip = mxc_nand_select_chip_v1_v3,
-	.correct_data = mxc_nand_correct_data_v1,
 	.enable_hwecc = mxc_nand_enable_hwecc_v1_v2,
 	.irqpending_quirk = 1,
 	.needs_ip = 0,
@@ -1629,7 +1600,6 @@ static const struct mxc_nand_devtype_data imx27_nand_devtype_data = {
 	.get_ecc_status = get_ecc_status_v1,
 	.ooblayout = &mxc_v1_ooblayout_ops,
 	.select_chip = mxc_nand_select_chip_v1_v3,
-	.correct_data = mxc_nand_correct_data_v1,
 	.enable_hwecc = mxc_nand_enable_hwecc_v1_v2,
 	.irqpending_quirk = 0,
 	.needs_ip = 0,
@@ -1655,7 +1625,6 @@ static const struct mxc_nand_devtype_data imx25_nand_devtype_data = {
 	.get_ecc_status = get_ecc_status_v2,
 	.ooblayout = &mxc_v2_ooblayout_ops,
 	.select_chip = mxc_nand_select_chip_v2,
-	.correct_data = mxc_nand_correct_data_v2_v3,
 	.setup_data_interface = mxc_nand_v2_setup_data_interface,
 	.enable_hwecc = mxc_nand_enable_hwecc_v1_v2,
 	.irqpending_quirk = 0,
@@ -1682,7 +1651,6 @@ static const struct mxc_nand_devtype_data imx51_nand_devtype_data = {
 	.get_ecc_status = get_ecc_status_v3,
 	.ooblayout = &mxc_v2_ooblayout_ops,
 	.select_chip = mxc_nand_select_chip_v1_v3,
-	.correct_data = mxc_nand_correct_data_v2_v3,
 	.enable_hwecc = mxc_nand_enable_hwecc_v3,
 	.irqpending_quirk = 0,
 	.needs_ip = 1,
@@ -1709,7 +1677,6 @@ static const struct mxc_nand_devtype_data imx53_nand_devtype_data = {
 	.get_ecc_status = get_ecc_status_v3,
 	.ooblayout = &mxc_v2_ooblayout_ops,
 	.select_chip = mxc_nand_select_chip_v1_v3,
-	.correct_data = mxc_nand_correct_data_v2_v3,
 	.enable_hwecc = mxc_nand_enable_hwecc_v3,
 	.irqpending_quirk = 0,
 	.needs_ip = 1,
@@ -1963,9 +1930,6 @@ static int mxcnd_probe(struct platform_device *pdev)
 		this->ecc.write_page = mxc_nand_write_page;
 		this->ecc.write_page_raw = mxc_nand_write_page_raw;
 		this->ecc.write_oob = mxc_nand_write_oob;
-		this->ecc.calculate = mxc_nand_calculate_ecc;
-		this->ecc.hwctl = mxc_nand_enable_hwecc;
-		this->ecc.correct = host->devtype_data->correct_data;
 		break;
 
 	case NAND_ECC_SOFT:
-- 
2.11.0

[PATCH 8/8] mtd: nand: mxc: remove now unused code

[Sascha Hauer]

Since we now have our own read_page/write_page functions
mxc_nand_command() will no longer be called with NAND_CMD_READ0,
NAND_CMD_READOOB, NAND_CMD_SEQIN and NAND_CMD_PAGEPROG. Remove
the code handling these commands.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/mxc_nand.c | 51 ---------------------------------------------
 1 file changed, 51 deletions(-)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 1d2c55e5ca89..efe29a179c5c 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -1403,57 +1403,6 @@ static void mxc_nand_command(struct mtd_info *mtd, unsigned command,
 		mxc_do_addr_cycle(mtd, column, page_addr);
 		break;
 
-	case NAND_CMD_READ0:
-	case NAND_CMD_READOOB:
-		if (command == NAND_CMD_READ0)
-			host->buf_start = column;
-		else
-			host->buf_start = column + mtd->writesize;
-
-		command = NAND_CMD_READ0; /* only READ0 is valid */
-
-		host->devtype_data->send_cmd(host, command, false);
-		WARN_ONCE(column < 0,
-			  "Unexpected column/row value (cmd=%u, col=%d, row=%d)\n",
-			  command, column, page_addr);
-		mxc_do_addr_cycle(mtd, 0, page_addr);
-
-		if (mtd->writesize > 512)
-			host->devtype_data->send_cmd(host,
-					NAND_CMD_READSTART, true);
-
-		host->devtype_data->send_page(mtd, NFC_OUTPUT);
-
-		memcpy32_fromio(host->data_buf, host->main_area0,
-				mtd->writesize);
-		copy_spare(mtd, true, host->data_buf + mtd->writesize);
-		break;
-
-	case NAND_CMD_SEQIN:
-		if (column >= mtd->writesize)
-			/* call ourself to read a page */
-			mxc_nand_command(mtd, NAND_CMD_READ0, 0, page_addr);
-
-		host->buf_start = column;
-
-		host->devtype_data->send_cmd(host, command, false);
-		WARN_ONCE(column < -1,
-			  "Unexpected column/row value (cmd=%u, col=%d, row=%d)\n",
-			  command, column, page_addr);
-		mxc_do_addr_cycle(mtd, 0, page_addr);
-		break;
-
-	case NAND_CMD_PAGEPROG:
-		memcpy32_toio(host->main_area0, host->data_buf, mtd->writesize);
-		copy_spare(mtd, false, host->data_buf + mtd->writesize);
-		host->devtype_data->send_page(mtd, NFC_INPUT);
-		host->devtype_data->send_cmd(host, command, true);
-		WARN_ONCE(column != -1 || page_addr != -1,
-			  "Unexpected column/row value (cmd=%u, col=%d, row=%d)\n",
-			  command, column, page_addr);
-		mxc_do_addr_cycle(mtd, column, page_addr);
-		break;
-
 	case NAND_CMD_READID:
 		host->devtype_data->send_cmd(host, command, true);
 		mxc_do_addr_cycle(mtd, column, page_addr);
-- 
2.11.0

Re: [PATCH 5/8] mtd: nand: mxc: Fix failed/corrected values for v1 controllers

[Boris Brezillon]

On Tue,  9 Jan 2018 11:11:45 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> The v1 controller code has several flaws:
> - We do not forward the number of corrected bitflips to the upper layers
> - For 2k page NAND chips only the status results from the fourth subpage
>   read are evaluated, so ECC failures in the other subpages remain
>   uncovered
> - When there are uncorrectable errors we have to increase the statistics
>   counter, but still have to return successfully. Currently we return
>   an error
> 
> This patch fixes this by introducing a v1 specific read_page function.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mtd/nand/mxc_nand.c | 78 +++++++++++++++++++++++++++++++++++----------
>  1 file changed, 62 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index d698f7c6666c..83bb01ab7079 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -738,22 +738,68 @@ static void mxc_nand_enable_hwecc(struct mtd_info *mtd, int mode)
>  static int mxc_nand_correct_data_v1(struct mtd_info *mtd, u_char *dat,
>  				 u_char *read_ecc, u_char *calc_ecc)
>  {
> +	return 0;
> +}
> +
> +static int mxc_nand_read_page_v1(struct mtd_info *mtd, struct nand_chip *chip,
> +				 void *buf, void *oob, bool ecc, int page)
> +{
>  	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> -	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +	unsigned int max_bitflips = 0;
> +	int no_subpages;
> +	int i;
>  
> -	/*
> -	 * 1-Bit errors are automatically corrected in HW.  No need for
> -	 * additional correction.  2-Bit errors cannot be corrected by
> -	 * HW ECC, so we need to return failure
> -	 */
> -	uint16_t ecc_status = get_ecc_status_v1(host);
> +	host->devtype_data->enable_hwecc(nand_chip, ecc);
> +
> +	host->devtype_data->send_cmd(host, NAND_CMD_READ0, false);
> +	mxc_do_addr_cycle(mtd, 0, page);
> +
> +	if (mtd->writesize > 512)
> +		host->devtype_data->send_cmd(host, NAND_CMD_READSTART, true);
>  
> -	if (((ecc_status & 0x3) == 2) || ((ecc_status >> 2) == 2)) {
> -		dev_dbg(host->dev, "HWECC uncorrectable 2-bit ECC error\n");
> -		return -EBADMSG;
> +	no_subpages = mtd->writesize >> 9;
> +
> +	for (i = 0; i < no_subpages; i++) {
> +		int flips = 0;
> +		uint16_t ecc_stats;
> +
> +		/* NANDFC buffer 0 is used for page read/write */
> +		writew((host->active_cs << 4) | i, NFC_V1_V2_BUF_ADDR);
> +
> +		writew(NFC_OUTPUT, NFC_V1_V2_CONFIG2);
> +
> +		/* Wait for operation to complete */
> +		wait_op_done(host, true);
> +
> +		ecc_stats = get_ecc_status_v1(host);
> +
> +		ecc_stats >>= 2;
> +
> +		if (buf && ecc) {
> +			switch (ecc_stats & 0x3) {
> +			case 0:
> +			default:
> +				break;
> +			case 1:
> +				mtd->ecc_stats.corrected++;
> +				flips++;

Since the engine is anyway able to correct a maximum of 1 bitflip you
can get rid of flips and do:

				max_bitflips = 1;


You could even rename max_bitflips into bitflips_corrected.

> +				break;
> +			case 2:
> +				mtd->ecc_stats.failed++;
> +				break;
> +			}
> +		}
> +
> +		max_bitflips = max_t(unsigned int, max_bitflips, flips);

If you get rid of flips, this is not needed.

>  	}
>  
> -	return 0;
> +	if (buf)
> +		memcpy32_fromio(buf, host->main_area0, mtd->writesize);
> +	if (oob)
> +		copy_spare(mtd, true, oob);
> +
> +	return max_bitflips;
>  }
>  
>  static int mxc_nand_correct_data_v2_v3(struct mtd_info *mtd, u_char *dat,
> @@ -1494,6 +1540,7 @@ static struct nand_bbt_descr bbt_mirror_descr = {
>  /* v1 + irqpending_quirk: i.MX21 */
>  static const struct mxc_nand_devtype_data imx21_nand_devtype_data = {
>  	.preset = preset_v1,
> +	.read_page = mxc_nand_read_page_v1,
>  	.send_cmd = send_cmd_v1_v2,
>  	.send_addr = send_addr_v1_v2,
>  	.send_page = send_page_v1,
> @@ -1518,6 +1565,7 @@ static const struct mxc_nand_devtype_data imx21_nand_devtype_data = {
>  /* v1 + !irqpending_quirk: i.MX27, i.MX31 */
>  static const struct mxc_nand_devtype_data imx27_nand_devtype_data = {
>  	.preset = preset_v1,
> +	.read_page = mxc_nand_read_page_v1,
>  	.send_cmd = send_cmd_v1_v2,
>  	.send_addr = send_addr_v1_v2,
>  	.send_page = send_page_v1,
> @@ -1856,11 +1904,9 @@ static int mxcnd_probe(struct platform_device *pdev)
>  
>  	switch (this->ecc.mode) {
>  	case NAND_ECC_HW:
> -		if (host->devtype_data->read_page) {
> -			this->ecc.read_page = mxc_nand_read_page;
> -			this->ecc.read_page_raw = mxc_nand_read_page_raw;
> -			this->ecc.read_oob = mxc_nand_read_oob;
> -		}
> +		this->ecc.read_page = mxc_nand_read_page;
> +		this->ecc.read_page_raw = mxc_nand_read_page_raw;
> +		this->ecc.read_oob = mxc_nand_read_oob;
>  		this->ecc.calculate = mxc_nand_calculate_ecc;
>  		this->ecc.hwctl = mxc_nand_enable_hwecc;
>  		this->ecc.correct = host->devtype_data->correct_data;

Re: [PATCH 4/8] mtd: nand: mxc: Fix failed/corrected values for v2/v3 controllers

[Boris Brezillon]

On Tue,  9 Jan 2018 11:11:44 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Currently nand_read_page_hwecc() from nand_base calls
> mxc_nand_correct_data_v2_v3() for each subpage, but in this function we
> return the corrected/failed results for the whole page instead
> of a single subpage. On a 2k page size Nand this leads to results which
> are 4 times too high.
> The whole ecc.calculate/ecc.correct mechanism used by
> nand_read_page_hwecc() is not suitable for devices which correct the
> data in hardware, so fix this by using a driver specific read_page
> function which does the right thing. Also add read_page_raw and read_oob
> For proper raw and oob read support.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mtd/nand/mxc_nand.c | 84 ++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 76 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index ab9cd45237d3..d698f7c6666c 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -140,6 +140,8 @@ struct mxc_nand_host;
>  
>  struct mxc_nand_devtype_data {
>  	void (*preset)(struct mtd_info *);
> +	int (*read_page)(struct mtd_info *mtd, struct nand_chip *chip,

You really don't have to pass both mtd and chip, since mtd can be
extraced from chip. Just pass chip and you should be good (the same
comment apply to other patches).

> +			void *buf, void *oob, bool ecc, int page);
>  	void (*send_cmd)(struct mxc_nand_host *, uint16_t, int);
>  	void (*send_addr)(struct mxc_nand_host *, uint16_t, int);
>  	void (*send_page)(struct mtd_info *, unsigned int);
> @@ -757,13 +759,35 @@ static int mxc_nand_correct_data_v1(struct mtd_info *mtd, u_char *dat,
>  static int mxc_nand_correct_data_v2_v3(struct mtd_info *mtd, u_char *dat,
>  				 u_char *read_ecc, u_char *calc_ecc)
>  {
> +	return 0;
> +}
> +
> +static int mxc_nand_read_page_v2_v3(struct mtd_info *mtd, struct nand_chip *chip,
> +				    void *buf, void *oob, bool ecc, int page)
> +{
>  	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> -	struct mxc_nand_host *host = nand_get_controller_data(nand_chip);
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +	unsigned int max_bitflips = 0;
>  	u32 ecc_stat, err;
> -	int no_subpages = 1;
> -	int ret = 0;
> +	int no_subpages;
>  	u8 ecc_bit_mask, err_limit;
>  
> +	host->devtype_data->enable_hwecc(nand_chip, ecc);
> +
> +	host->devtype_data->send_cmd(host, NAND_CMD_READ0, false);
> +	mxc_do_addr_cycle(mtd, 0, page);
> +
> +	if (mtd->writesize > 512)
> +		host->devtype_data->send_cmd(host,
> +				NAND_CMD_READSTART, true);
> +
> +	host->devtype_data->send_page(mtd, NFC_OUTPUT);
> +
> +	if (buf)
> +		memcpy32_fromio(buf, host->main_area0, mtd->writesize);
> +	if (oob)
> +		copy_spare(mtd, true, oob);
> +
>  	ecc_bit_mask = (host->eccsize == 4) ? 0x7 : 0xf;
>  	err_limit = (host->eccsize == 4) ? 0x4 : 0x8;
>  
> @@ -774,17 +798,53 @@ static int mxc_nand_correct_data_v2_v3(struct mtd_info *mtd, u_char *dat,
>  	do {
>  		err = ecc_stat & ecc_bit_mask;
>  		if (err > err_limit) {
> -			dev_dbg(host->dev, "UnCorrectable RS-ECC Error\n");
> -			return -EBADMSG;
> +			mtd->ecc_stats.failed++;
>  		} else {
> -			ret += err;
> +			mtd->ecc_stats.corrected += err;
> +			max_bitflips = max_t(unsigned int, max_bitflips, err);
>  		}
> +
>  		ecc_stat >>= 4;
>  	} while (--no_subpages);
>  
> -	dev_dbg(host->dev, "%d Symbol Correctable RS-ECC Error\n", ret);
> +	return max_bitflips;
> +}
>  
> -	return ret;
> +static int mxc_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
> +			      uint8_t *buf, int oob_required, int page)
> +{
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +	void *oob_buf;
> +
> +	if (oob_required)
> +		oob_buf = chip->oob_poi;
> +	else
> +		oob_buf = NULL;
> +
> +	return host->devtype_data->read_page(mtd, chip, buf, oob_buf, 1, page);
> +}
> +
> +static int mxc_nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> +				  uint8_t *buf, int oob_required, int page)
> +{
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +	void *oob_buf;
> +
> +	if (oob_required)
> +		oob_buf = chip->oob_poi;
> +	else
> +		oob_buf = NULL;
> +
> +	return host->devtype_data->read_page(mtd, chip, buf, oob_buf, 0, page);
> +}
> +
> +static int mxc_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
> +			     int page)
> +{
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	return host->devtype_data->read_page(mtd, chip, NULL, chip->oob_poi, 0,
> +					     page);
>  }
>  
>  static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
> @@ -1483,6 +1543,7 @@ static const struct mxc_nand_devtype_data imx27_nand_devtype_data = {
>  /* v21: i.MX25, i.MX35 */
>  static const struct mxc_nand_devtype_data imx25_nand_devtype_data = {
>  	.preset = preset_v2,
> +	.read_page = mxc_nand_read_page_v2_v3,
>  	.send_cmd = send_cmd_v1_v2,
>  	.send_addr = send_addr_v1_v2,
>  	.send_page = send_page_v2,
> @@ -1509,6 +1570,7 @@ static const struct mxc_nand_devtype_data imx25_nand_devtype_data = {
>  /* v3.2a: i.MX51 */
>  static const struct mxc_nand_devtype_data imx51_nand_devtype_data = {
>  	.preset = preset_v3,
> +	.read_page = mxc_nand_read_page_v2_v3,
>  	.send_cmd = send_cmd_v3,
>  	.send_addr = send_addr_v3,
>  	.send_page = send_page_v3,
> @@ -1535,6 +1597,7 @@ static const struct mxc_nand_devtype_data imx51_nand_devtype_data = {
>  /* v3.2b: i.MX53 */
>  static const struct mxc_nand_devtype_data imx53_nand_devtype_data = {
>  	.preset = preset_v3,
> +	.read_page = mxc_nand_read_page_v2_v3,
>  	.send_cmd = send_cmd_v3,
>  	.send_addr = send_addr_v3,
>  	.send_page = send_page_v3,
> @@ -1793,6 +1856,11 @@ static int mxcnd_probe(struct platform_device *pdev)
>  
>  	switch (this->ecc.mode) {
>  	case NAND_ECC_HW:
> +		if (host->devtype_data->read_page) {
> +			this->ecc.read_page = mxc_nand_read_page;
> +			this->ecc.read_page_raw = mxc_nand_read_page_raw;
> +			this->ecc.read_oob = mxc_nand_read_oob;
> +		}
>  		this->ecc.calculate = mxc_nand_calculate_ecc;
>  		this->ecc.hwctl = mxc_nand_enable_hwecc;
>  		this->ecc.correct = host->devtype_data->correct_data;

Re: [PATCH 6/8] mtd: nand: mxc: Add own write_page

[Boris Brezillon]

On Tue,  9 Jan 2018 11:11:46 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Now that we have our own read_page function add a write_page function
> for consistency aswell. This can be a lot easier than the generic
> function since we do not have to iterate over subpages but can write
> the whole page at once. Also add write_page_raw and write_oob for
> proper raw and oob write support.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mtd/nand/mxc_nand.c | 56 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index 83bb01ab7079..6fbf98851a96 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -893,6 +893,59 @@ static int mxc_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
>  					     page);
>  }
>  
> +static int __mxc_nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,

I'm not a big fan of those '__' prefixes. How about keeping
mxc_nand_write_page() for this one, renaming the other
mxc_nand_write_page() into mxc_nand_write_page_ecc() and keeping the
other ones unchanged.

> +				 const uint8_t *buf, int oob_required, int page)

Why don't you pass a 'bool enable_ecc' argument here so that you can
directly do

	host->devtype_data->enable_hwecc(nand_chip, enable_ecc);

from here instead of duplicating it in all wrappers?

> +{
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	host->devtype_data->send_cmd(host, NAND_CMD_SEQIN, false);
> +	mxc_do_addr_cycle(mtd, 0, page);
> +
> +	memcpy32_toio(host->main_area0, buf, mtd->writesize);
> +	copy_spare(mtd, false, chip->oob_poi);
> +
> +	host->devtype_data->send_page(mtd, NFC_INPUT);
> +	host->devtype_data->send_cmd(host, NAND_CMD_PAGEPROG, true);
> +	mxc_do_addr_cycle(mtd, 0, page);
> +
> +	return 0;
> +}
> +
> +static int mxc_nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
> +			       const uint8_t *buf, int oob_required, int page)
> +{
> +	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	host->devtype_data->enable_hwecc(nand_chip, true);
> +
> +	return __mxc_nand_write_page(mtd, chip, buf, oob_required, page);
> +}
> +
> +static int mxc_nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
> +				   const uint8_t *buf, int oob_required, int page)
> +{
> +	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	host->devtype_data->enable_hwecc(nand_chip, false);
> +
> +	return __mxc_nand_write_page(mtd, chip, buf, oob_required, page);
> +}
> +
> +static int mxc_nand_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
> +			      int page)
> +{
> +	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	memset(host->data_buf, 0xff, mtd->writesize);
> +
> +	host->devtype_data->enable_hwecc(nand_chip, false);
> +
> +	return __mxc_nand_write_page(mtd, chip, host->data_buf, 1, page);
> +}
> +
>  static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
>  				  u_char *ecc_code)
>  {
> @@ -1907,6 +1960,9 @@ static int mxcnd_probe(struct platform_device *pdev)
>  		this->ecc.read_page = mxc_nand_read_page;
>  		this->ecc.read_page_raw = mxc_nand_read_page_raw;
>  		this->ecc.read_oob = mxc_nand_read_oob;
> +		this->ecc.write_page = mxc_nand_write_page;
> +		this->ecc.write_page_raw = mxc_nand_write_page_raw;
> +		this->ecc.write_oob = mxc_nand_write_oob;
>  		this->ecc.calculate = mxc_nand_calculate_ecc;
>  		this->ecc.hwctl = mxc_nand_enable_hwecc;
>  		this->ecc.correct = host->devtype_data->correct_data;

Re: [PATCH 6/8] mtd: nand: mxc: Add own write_page

[Miquel Raynal]

Hi Sascha,

On Tue,  9 Jan 2018 11:11:46 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Now that we have our own read_page function add a write_page function
> for consistency aswell. This can be a lot easier than the generic
> function since we do not have to iterate over subpages but can write
> the whole page at once. Also add write_page_raw and write_oob for
> proper raw and oob write support.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mtd/nand/mxc_nand.c | 56
> +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56
> insertions(+)
> 
> diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> index 83bb01ab7079..6fbf98851a96 100644
> --- a/drivers/mtd/nand/mxc_nand.c
> +++ b/drivers/mtd/nand/mxc_nand.c
> @@ -893,6 +893,59 @@ static int mxc_nand_read_oob(struct mtd_info
> *mtd, struct nand_chip *chip, page);
>  }
>  
> +static int __mxc_nand_write_page(struct mtd_info *mtd, struct
> nand_chip *chip,
> +				 const uint8_t *buf, int
> oob_required, int page) +{
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	host->devtype_data->send_cmd(host, NAND_CMD_SEQIN, false);
> +	mxc_do_addr_cycle(mtd, 0, page);
> +
> +	memcpy32_toio(host->main_area0, buf, mtd->writesize);
> +	copy_spare(mtd, false, chip->oob_poi);

If I remember correctly, there was a "spare only" mechanism to avoid
doing full page read/write when unnecessary. What about using it here
when (buf == NULL && oob_required)?

> +
> +	host->devtype_data->send_page(mtd, NFC_INPUT);
> +	host->devtype_data->send_cmd(host, NAND_CMD_PAGEPROG, true);
> +	mxc_do_addr_cycle(mtd, 0, page);
> +
> +	return 0;
> +}
> +
> +static int mxc_nand_write_page(struct mtd_info *mtd, struct
> nand_chip *chip,
> +			       const uint8_t *buf, int oob_required,
> int page) +{
> +	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	host->devtype_data->enable_hwecc(nand_chip, true);
> +
> +	return __mxc_nand_write_page(mtd, chip, buf, oob_required,
> page); +}
> +
> +static int mxc_nand_write_page_raw(struct mtd_info *mtd, struct
> nand_chip *chip,
> +				   const uint8_t *buf, int
> oob_required, int page) +{
> +	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	host->devtype_data->enable_hwecc(nand_chip, false);
> +
> +	return __mxc_nand_write_page(mtd, chip, buf, oob_required,
> page); +}
> +
> +static int mxc_nand_write_oob(struct mtd_info *mtd, struct nand_chip
> *chip,
> +			      int page)
> +{
> +	struct nand_chip *nand_chip = mtd_to_nand(mtd);
> +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> +
> +	memset(host->data_buf, 0xff, mtd->writesize);

If the above solution works, this won't be needed anymore.

> +
> +	host->devtype_data->enable_hwecc(nand_chip, false);
> +
> +	return __mxc_nand_write_page(mtd, chip, host->data_buf, 1,
> page);

And this could be:
    return __mxc_nand_write_page(mtd, chip, NULL, 1, page);

> +}
> +
>  static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char
> *dat, u_char *ecc_code)
>  {
> @@ -1907,6 +1960,9 @@ static int mxcnd_probe(struct platform_device
> *pdev) this->ecc.read_page = mxc_nand_read_page;
>  		this->ecc.read_page_raw = mxc_nand_read_page_raw;
>  		this->ecc.read_oob = mxc_nand_read_oob;
> +		this->ecc.write_page = mxc_nand_write_page;
> +		this->ecc.write_page_raw = mxc_nand_write_page_raw;
> +		this->ecc.write_oob = mxc_nand_write_oob;
>  		this->ecc.calculate = mxc_nand_calculate_ecc;
>  		this->ecc.hwctl = mxc_nand_enable_hwecc;
>  		this->ecc.correct = host->devtype_data->correct_data;


Thanks,
Miquèl

Re: [PATCH 6/8] mtd: nand: mxc: Add own write_page

[Sascha Hauer]

Hi Miquel,

On Sat, Jan 13, 2018 at 05:25:51PM +0100, Miquel Raynal wrote:
> Hi Sascha,
> 
> On Tue,  9 Jan 2018 11:11:46 +0100
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Now that we have our own read_page function add a write_page function
> > for consistency aswell. This can be a lot easier than the generic
> > function since we do not have to iterate over subpages but can write
> > the whole page at once. Also add write_page_raw and write_oob for
> > proper raw and oob write support.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/mtd/nand/mxc_nand.c | 56
> > +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56
> > insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
> > index 83bb01ab7079..6fbf98851a96 100644
> > --- a/drivers/mtd/nand/mxc_nand.c
> > +++ b/drivers/mtd/nand/mxc_nand.c
> > @@ -893,6 +893,59 @@ static int mxc_nand_read_oob(struct mtd_info
> > *mtd, struct nand_chip *chip, page);
> >  }
> >  
> > +static int __mxc_nand_write_page(struct mtd_info *mtd, struct
> > nand_chip *chip,
> > +				 const uint8_t *buf, int
> > oob_required, int page) +{
> > +	struct mxc_nand_host *host = nand_get_controller_data(chip);
> > +
> > +	host->devtype_data->send_cmd(host, NAND_CMD_SEQIN, false);
> > +	mxc_do_addr_cycle(mtd, 0, page);
> > +
> > +	memcpy32_toio(host->main_area0, buf, mtd->writesize);
> > +	copy_spare(mtd, false, chip->oob_poi);
> 
> If I remember correctly, there was a "spare only" mechanism to avoid
> doing full page read/write when unnecessary. What about using it here
> when (buf == NULL && oob_required)?

You are probably referring to the SP_EN bit in the NAND_FLASH_CONFIG1
register right? This is described as:

> NAND Flash spare enable. This bit determines whether host reads/writes
> are to NAND Flash spare data only or NAND Flash main and spare data.
> This feature is supported only with memories with 1/2K page size.
> Note: There is no ECC in this mode of operation.
> 0 NAND Flash main and spare data is enabled
> 1 NAND Flash spare only data is enabled

At least the i.MX25 has the limitation of 1/2k page size for this
feature. It's not in the i.MX27 manual. I don't know if the i.MX27
documentation is inaccurate or if this limitation is there already.

Anyway, I don't think it's worth optimizing for this case given that
we still need to support a full page write for 2k pages (which should
be much more widespread nowadays than the 512b page size NANDs).

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 6/8] mtd: nand: mxc: Add own write_page

[Sascha Hauer]

Now that we have our own read_page function add a write_page function
for consistency aswell. This can be a lot easier than the generic
function since we do not have to iterate over subpages but can write
the whole page at once. Also add write_page_raw and write_oob for
proper raw and oob write support.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/mxc_nand.c | 47 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 49885afbcf43..2811102b0469 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -890,6 +890,50 @@ static int mxc_nand_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 					     page);
 }
 
+static int mxc_nand_write_page(struct nand_chip *chip, const uint8_t *buf,
+			       bool ecc, int page)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+
+	host->devtype_data->enable_hwecc(chip, ecc);
+
+	host->devtype_data->send_cmd(host, NAND_CMD_SEQIN, false);
+	mxc_do_addr_cycle(mtd, 0, page);
+
+	memcpy32_toio(host->main_area0, buf, mtd->writesize);
+	copy_spare(mtd, false, chip->oob_poi);
+
+	host->devtype_data->send_page(mtd, NFC_INPUT);
+	host->devtype_data->send_cmd(host, NAND_CMD_PAGEPROG, true);
+	mxc_do_addr_cycle(mtd, 0, page);
+
+	return 0;
+}
+
+static int mxc_nand_write_page_ecc(struct mtd_info *mtd, struct nand_chip *chip,
+				   const uint8_t *buf, int oob_required,
+				   int page)
+{
+	return mxc_nand_write_page(chip, buf, true, page);
+}
+
+static int mxc_nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
+				   const uint8_t *buf, int oob_required, int page)
+{
+	return mxc_nand_write_page(chip, buf, false, page);
+}
+
+static int mxc_nand_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
+			      int page)
+{
+	struct mxc_nand_host *host = nand_get_controller_data(chip);
+
+	memset(host->data_buf, 0xff, mtd->writesize);
+
+	return mxc_nand_write_page(chip, host->data_buf, false, page);
+}
+
 static int mxc_nand_calculate_ecc(struct mtd_info *mtd, const u_char *dat,
 				  u_char *ecc_code)
 {
@@ -1904,6 +1948,9 @@ static int mxcnd_probe(struct platform_device *pdev)
 		this->ecc.read_page = mxc_nand_read_page;
 		this->ecc.read_page_raw = mxc_nand_read_page_raw;
 		this->ecc.read_oob = mxc_nand_read_oob;
+		this->ecc.write_page = mxc_nand_write_page_ecc;
+		this->ecc.write_page_raw = mxc_nand_write_page_raw;
+		this->ecc.write_oob = mxc_nand_write_oob;
 		this->ecc.calculate = mxc_nand_calculate_ecc;
 		this->ecc.hwctl = mxc_nand_enable_hwecc;
 		this->ecc.correct = host->devtype_data->correct_data;
-- 
2.11.0

Re: [PATCH 6/8] mtd: nand: mxc: Add own write_page

[Miquel Raynal]

Hi Sascha,

On Wed, 17 Jan 2018 11:40:09 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Hi Miquel,
> 
> On Sat, Jan 13, 2018 at 05:25:51PM +0100, Miquel Raynal wrote:
> > Hi Sascha,
> > 
> > On Tue,  9 Jan 2018 11:11:46 +0100
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >   
> > > Now that we have our own read_page function add a write_page
> > > function for consistency aswell. This can be a lot easier than
> > > the generic function since we do not have to iterate over
> > > subpages but can write the whole page at once. Also add
> > > write_page_raw and write_oob for proper raw and oob write support.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > ---
> > >  drivers/mtd/nand/mxc_nand.c | 56
> > > +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56
> > > insertions(+)
> > > 
> > > diff --git a/drivers/mtd/nand/mxc_nand.c
> > > b/drivers/mtd/nand/mxc_nand.c index 83bb01ab7079..6fbf98851a96
> > > 100644 --- a/drivers/mtd/nand/mxc_nand.c
> > > +++ b/drivers/mtd/nand/mxc_nand.c
> > > @@ -893,6 +893,59 @@ static int mxc_nand_read_oob(struct mtd_info
> > > *mtd, struct nand_chip *chip, page);
> > >  }
> > >  
> > > +static int __mxc_nand_write_page(struct mtd_info *mtd, struct
> > > nand_chip *chip,
> > > +				 const uint8_t *buf, int
> > > oob_required, int page) +{
> > > +	struct mxc_nand_host *host =
> > > nand_get_controller_data(chip); +
> > > +	host->devtype_data->send_cmd(host, NAND_CMD_SEQIN,
> > > false);
> > > +	mxc_do_addr_cycle(mtd, 0, page);
> > > +
> > > +	memcpy32_toio(host->main_area0, buf, mtd->writesize);
> > > +	copy_spare(mtd, false, chip->oob_poi);  
> > 
> > If I remember correctly, there was a "spare only" mechanism to avoid
> > doing full page read/write when unnecessary. What about using it
> > here when (buf == NULL && oob_required)?  
> 
> You are probably referring to the SP_EN bit in the NAND_FLASH_CONFIG1
> register right? This is described as:
> 
> > NAND Flash spare enable. This bit determines whether host
> > reads/writes are to NAND Flash spare data only or NAND Flash main
> > and spare data. This feature is supported only with memories with
> > 1/2K page size. Note: There is no ECC in this mode of operation.
> > 0 NAND Flash main and spare data is enabled
> > 1 NAND Flash spare only data is enabled  
> 
> At least the i.MX25 has the limitation of 1/2k page size for this
> feature. It's not in the i.MX27 manual. I don't know if the i.MX27
> documentation is inaccurate or if this limitation is there already.

Ok, I ignored this limitation on i.MX25. Better write generic code
then.

> 
> Anyway, I don't think it's worth optimizing for this case given that
> we still need to support a full page write for 2k pages (which should
> be much more widespread nowadays than the 512b page size NANDs).

Sure!

Thanks,
Miquèl