mtd: mxc_nand: fix 8 bit ECC and large oob

Signed-off-by Uwe Kleine-K?nig <[email protected]>

[baruch: declare oob_chunk_size; update comments; reword commit log]

---
 drivers/mtd/nand/mxc_nand.c | 46 ++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

drivers/mtd/nand/mxc_nand.c +31 ~~-15~~

index 372e0e38f59b..33b22b9c0b30 100644
@@ -807,32 +807,48 @@ static void mxc_nand_select_chip_v2(struct mtd_info *mtd, int chip)
 }
 
 /*
- * Function to transfer data to/from spare area.
+ * 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->priv;
 	struct mxc_nand_host *host = this->priv;
-	u16 i, j;
-	u16 n = mtd->writesize >> 9;
+	u16 i, oob_chunk_size;
+	u16 num_chunks = mtd->writesize / 512;
+
 	u8 *d = host->data_buf + mtd->writesize;
 	u8 __iomem *s = host->spare0;
-	u16 t = host->devtype_data->spare_len;
+	u16 sparebuf_size = host->devtype_data->spare_len;
 
-	j = (mtd->oobsize / n >> 1) << 1;
+	/* size of oob chunk for all but possibly the last one */
+	oob_chunk_size = (mtd->oobsize / num_chunks) & ~1;
 
 	if (bfrom) {
-		for (i = 0; i < n - 1; i++)
-			memcpy32_fromio(d + i * j, s + i * t, j);
-
-		/* the last section */
-		memcpy32_fromio(d + i * j, s + i * t, mtd->oobsize - i * j);
+		for (i = 0; i < num_chunks - 1; i++)
+			memcpy32_fromio(d + i * oob_chunk_size,
+					s + i * sparebuf_size,
+					oob_chunk_size);
+
+		/* the last chunk */
+		memcpy32_fromio(d + i * oob_chunk_size,
+				s + i * sparebuf_size,
+				mtd->oobsize - i * oob_chunk_size);
 	} else {
-		for (i = 0; i < n - 1; i++)
-			memcpy32_toio(&s[i * t], &d[i * j], j);
-
-		/* the last section */
-		memcpy32_toio(&s[i * t], &d[i * j], mtd->oobsize - i * j);
+		for (i = 0; i < num_chunks - 1; i++)
+			memcpy32_toio(&s[i * sparebuf_size],
+				      &d[i * oob_chunk_size],
+				      oob_chunk_size);
+
+		/* the last chunk */
+		memcpy32_toio(&s[oob_chunk_size * sparebuf_size],
+			      &d[i * oob_chunk_size],
+			      mtd->oobsize - i * oob_chunk_size);
 	}
 }

Signature · 1 line

2.1.4

Baruch Siach

author

2015-05-13 08:17 UTC #2 lore raw

PATCH v3 2/4 mtd: mxc_nand: limit the size of used oob

For 4k pages the i.MX NFC hardware uses no more than 218 bytes for 8bit ECC data. Larger oobsize confuses the logic of copy_spare(). Limit the size of used oob size to avoid that.

Acked-by Uwe Kleine-K?nig <[email protected]>

---
v2:
* Move used_oobsize into struct mxc_nand_host, and initialize it from

     _probe() (Uwe Kleine-K?nig)
   * Add a more details to in-code comment
---
 drivers/mtd/nand/mxc_nand.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

drivers/mtd/nand/mxc_nand.c +13 -3

index 33b22b9c0b30..51c1600d7eb9 100644
@@ -189,6 +189,7 @@ struct mxc_nand_host {
 	int			clk_act;
 	int			irq;
 	int			eccsize;
+	int			used_oobsize;
 	int			active_cs;
 
 	struct completion	op_completion;

@@ -827,7 +828,7 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom)
 	u16 sparebuf_size = host->devtype_data->spare_len;
 
 	/* size of oob chunk for all but possibly the last one */
-	oob_chunk_size = (mtd->oobsize / num_chunks) & ~1;
+	oob_chunk_size = (host->used_oobsize / num_chunks) & ~1;
 
 	if (bfrom) {
 		for (i = 0; i < num_chunks - 1; i++)

@@ -838,7 +839,7 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom)
 		/* the last chunk */
 		memcpy32_fromio(d + i * oob_chunk_size,
 				s + i * sparebuf_size,
-				mtd->oobsize - i * oob_chunk_size);
+				host->used_oobsize - i * oob_chunk_size);
 	} else {
 		for (i = 0; i < num_chunks - 1; i++)
 			memcpy32_toio(&s[i * sparebuf_size],

@@ -848,7 +849,7 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom)
 		/* the last chunk */
 		memcpy32_toio(&s[oob_chunk_size * sparebuf_size],
 			      &d[i * oob_chunk_size],
-			      mtd->oobsize - i * oob_chunk_size);
+			      host->used_oobsize - i * oob_chunk_size);
 	}
 }

@@ -1606,6 +1607,15 @@ static int mxcnd_probe(struct platform_device *pdev)
 	else if (mtd->writesize == 4096)
 		this->ecc.layout = host->devtype_data->ecclayout_4k;
 
+	/*
+	 * Experimentation shows that i.MX NFC can only handle up to 218 oob
+	 * bytes. Limit used_oobsize to 218 so as to not confuse copy_spare()
+	 * into copying invalid data to/from the spare IO buffer, as this
+	 * might cause ECC data corruption when doing sub-page write to a
+	 * partially written page.
+	 */
+	host->used_oobsize = min(mtd->oobsize, 218U);
+
 	if (this->ecc.mode == NAND_ECC_HW) {
 		if (is_imx21_nfc(host) || is_imx27_nfc(host))
 			this->ecc.strength = 1;

Signature · 1 line

2.1.4

Baruch Siach

author

2015-05-13 08:17 UTC #3 lore raw

PATCH v3 3/4 mtd: mxc_nand: fix truncate of unaligned oob copying

Copy to/from oob io area might not be aligned to 4 bytes. When 8 bit ECC is used, the buffer size is 26. Add memcpy16_{to,from}io, and use them to avoid truncating the buffer. Prefer memcpy32_{to,from}io when the buffer is properly aligned for better performance.

Acked-by Uwe Kleine-K?nig <[email protected]>

---
v2:
* Use memcpy16_{to,from}io (Uwe Kleine-K?nig)

---
 drivers/mtd/nand/mxc_nand.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

drivers/mtd/nand/mxc_nand.c +36 -4

index 51c1600d7eb9..010be8aa41d4 100644
@@ -281,12 +281,44 @@ static void memcpy32_fromio(void *trg, const void __iomem  *src, size_t size)
 		*t++ = __raw_readl(s++);
 }
 
+static void memcpy16_fromio(void *trg, const void __iomem  *src, size_t size)
+{
+	int i;
+	u16 *t = trg;
+	const __iomem u16 *s = src;
+
+	/* We assume that src (IO) is always 32bit aligned */
+	if (PTR_ALIGN(trg, 4) == trg && IS_ALIGNED(size, 4)) {
+		memcpy32_fromio(trg, src, size);
+		return;
+	}
+
+	for (i = 0; i < (size >> 1); i++)
+		*t++ = __raw_readw(s++);
+}
+
 static inline void memcpy32_toio(void __iomem *trg, const void *src, int size)
 {
 	/* __iowrite32_copy use 32bit size values so divide by 4 */
 	__iowrite32_copy(trg, src, size / 4);
 }
 
+static void memcpy16_toio(void __iomem *trg, const void *src, int size)
+{
+	int i;
+	__iomem u16 *t = trg;
+	const u16 *s = src;
+
+	/* We assume that trg (IO) is always 32bit aligned */
+	if (PTR_ALIGN(src, 4) == src && IS_ALIGNED(size, 4)) {
+		memcpy32_toio(trg, src, size);
+		return;
+	}
+
+	for (i = 0; i < (size >> 1); i++)
+		__raw_writew(*s++, t++);
+}
+
 static int check_int_v3(struct mxc_nand_host *host)
 {
 	uint32_t tmp;

@@ -832,22 +864,22 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom)
 
 	if (bfrom) {
 		for (i = 0; i < num_chunks - 1; i++)
-			memcpy32_fromio(d + i * oob_chunk_size,
+			memcpy16_fromio(d + i * oob_chunk_size,
 					s + i * sparebuf_size,
 					oob_chunk_size);
 
 		/* the last chunk */
-		memcpy32_fromio(d + i * oob_chunk_size,
+		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++)
-			memcpy32_toio(&s[i * sparebuf_size],
+			memcpy16_toio(&s[i * sparebuf_size],
 				      &d[i * oob_chunk_size],
 				      oob_chunk_size);
 
 		/* the last chunk */
-		memcpy32_toio(&s[oob_chunk_size * sparebuf_size],
+		memcpy16_toio(&s[oob_chunk_size * sparebuf_size],
 			      &d[i * oob_chunk_size],
 			      host->used_oobsize - i * oob_chunk_size);
 	}

Signature · 1 line

2.1.4

Baruch Siach

author

2015-05-13 08:17 UTC #4 lore raw

PATCH v3 4/4 mtd: mxc_nand: generate nand_ecclayout for 8 bit ECC

Hardware 8 bit ECC requires a different nand_ecclayout. Instead of adding yet another static struct nand_ecclayout, generate it in code.

---
v3:
Rename ecc_8bit_layout to ecc_8bit_layout_4k (Uwe Kleine-K?nig)

v2:
Initialize eccbytes

---
 drivers/mtd/nand/mxc_nand.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

drivers/mtd/nand/mxc_nand.c +21 -1

index 010be8aa41d4..25759563bf11 100644
@@ -960,6 +960,23 @@ static int get_eccsize(struct mtd_info *mtd)
 		return 8;
 }
 
+static void ecc_8bit_layout_4k(struct nand_ecclayout *layout)
+{
+	int i, j;
+
+	layout->eccbytes = 8*18;
+	for (i = 0; i < 8; i++)
+		for (j = 0; j < 18; j++)
+			layout->eccpos[i*18 + j] = i*26 + j + 7;
+
+	layout->oobfree[0].offset = 2;
+	layout->oobfree[0].length = 4;
+	for (i = 1; i < 8; i++) {
+		layout->oobfree[i].offset = i*26;
+		layout->oobfree[i].length = 7;
+	}
+}
+
 static void preset_v1(struct mtd_info *mtd)
 {
 	struct nand_chip *nand_chip = mtd->priv;

@@ -1636,8 +1653,11 @@ static int mxcnd_probe(struct platform_device *pdev)
 
 	if (mtd->writesize == 2048)
 		this->ecc.layout = host->devtype_data->ecclayout_2k;
-	else if (mtd->writesize == 4096)
+	else if (mtd->writesize == 4096) {
 		this->ecc.layout = host->devtype_data->ecclayout_4k;
+		if (get_eccsize(mtd) == 8)
+			ecc_8bit_layout_4k(this->ecc.layout);
+	}
 
 	/*
 	 * Experimentation shows that i.MX NFC can only handle up to 218 oob

Signature · 1 line

2.1.4

Uwe Kleine-König

2015-05-13 08:24 UTC #5 lore raw

PATCH v3 4/4 mtd: mxc_nand: generate nand_ecclayout for 8 bit ECC

Quoted 5 lines · Baruch Siach

Hardware 8 bit ECC requires a different nand_ecclayout. Instead of adding yet another static struct nand_ecclayout, generate it in code.
Reviewed-by Sascha Hauer <[email protected]>
Signed-off-by Baruch Siach <[email protected]>

Acked-by Uwe Kleine-K?nig <[email protected]>

Thanks
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Brian Norris

2015-05-20 22:38 UTC #9 lore raw

v3 cover mtd: mxc_nand: fix 8 bit ECC and large oob

Quoted 13 lines · Baruch Siach

This patch series fixes mxc_nand bugs that I encountered while trying to use 8 bit ECC mode on i.MX25 with the Micron MT29F8G08ABABA flash chip.
Many thanks to Uwe Kleine-K?nig for guiding me through the process of debugging and fixing, for contributing a clean-up patch to this series, and for reviewing the rest.
This series applies on v4.1-rc1.
This is version 3 of the patch series, adding Reviewed-by and Acked-by tags from Sascha Hauer and Uwe Kleine-K?nig. Patch 4/4 renamed a function as suggested by Uwe Kleine-K?nig.

Quoted 5 lines

Version 2 of the patch series
(http://thread.gmane.org/gmane.linux.drivers.mtd/58687), addressed the comments and suggestion of Uwe Kleine-K?nig on the first series
(http://thread.gmane.org/gmane.linux.ports.arm.kernel/408635). See each patch for detailed per-patch changelog.

Pushed the first 3 to l2-mtd.git. Thanks all!

I have one comment on the 4th.

Brian

Brian Norris

2015-05-20 22:41 UTC #6 lore raw

PATCH v3 4/4 mtd: mxc_nand: generate nand_ecclayout for 8 bit ECC

Quoted 2 lines · Baruch Siach

Hardware 8 bit ECC requires a different nand_ecclayout. Instead of adding yet another static struct nand_ecclayout, generate it in code.

I like the idea of not adding more static declarations. But I think you have a potential problem below.

Quoted 41 lines