[PATCH 5/6] mtd: nand: gpmi: correct bitflip for erased NAND page

[PATCH 5/6] mtd: nand: gpmi: correct bitflip for erased NAND page

[Han Xu]

i.MX6QP and i.MX7D BCH module integrated a new feature to detect the
bitflip number for erased NAND page. So for these two platform, set the
erase threshold to gf/2 and if bitflip detected, GPMI driver will
correct the data to all 0xFF.

Signed-off-by: Han Xu <b45815@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/bch-regs.h  | 10 ++++++++++
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |  5 +++++
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 24 +++++++++++++++++++++++-
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h |  5 ++++-
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/bch-regs.h b/drivers/mtd/nand/gpmi-nand/bch-regs.h
index 53e58bc..a84d72b 100644
--- a/drivers/mtd/nand/gpmi-nand/bch-regs.h
+++ b/drivers/mtd/nand/gpmi-nand/bch-regs.h
@@ -30,7 +30,13 @@
 #define BM_BCH_CTRL_COMPLETE_IRQ		(1 << 0)
 
 #define HW_BCH_STATUS0				0x00000010
+
 #define HW_BCH_MODE				0x00000020
+#define BP_BCH_MODE_ERASE_THRESHOLD		0
+#define BM_BCH_MODE_ERASE_THRESHOLD	(0xff << BP_BCH_MODE_ERASE_THRESHOLD)
+#define BF_BCH_MODE_ERASE_THRESHOLD(v)		\
+	(((v) << BP_BCH_MODE_ERASE_THRESHOLD) & BM_BCH_MODE_ERASE_THRESHOLD)
+
 #define HW_BCH_ENCODEPTR			0x00000030
 #define HW_BCH_DATAPTR				0x00000040
 #define HW_BCH_METAPTR				0x00000050
@@ -125,4 +131,8 @@
 	)
 
 #define HW_BCH_VERSION				0x00000160
+#define HW_BCH_DEBUG1				0x00000170
+#define BP_BCH_DEBUG1_ERASED_ZERO_COUNT	0
+#define BM_BCH_DEBUG1_ERASED_ZERO_COUNT		\
+		(0x1ff << BP_BCH_DEBUG1_ERASED_ZERO_COUNT)
 #endif
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
index 6b37414..44433da 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
@@ -298,6 +298,11 @@ int bch_set_geometry(struct gpmi_nand_data *this)
 			| BF_BCH_FLASH0LAYOUT1_DATAN_SIZE(block_size, this),
 			r->bch_regs + HW_BCH_FLASH0LAYOUT1);
 
+	/* Set erase threshold to gf/2 for mx6qp and mx7 */
+	if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this))
+		writel(BF_BCH_MODE_ERASE_THRESHOLD(gf_len/2),
+			r->bch_regs + HW_BCH_MODE);
+
 	/* Set *all* chip selects to use layout 0. */
 	writel(0, r->bch_regs + HW_BCH_LAYOUTSELECT);
 
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 0462103..362cf4c 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -71,6 +71,12 @@ static const struct gpmi_devdata gpmi_devdata_imx6q = {
 	.max_chain_delay = 12,
 };
 
+static const struct gpmi_devdata gpmi_devdata_imx6qp = {
+	.type = IS_MX6QP,
+	.bch_max_ecc_strength = 40,
+	.max_chain_delay = 12,
+};
+
 static const struct gpmi_devdata gpmi_devdata_imx6sx = {
 	.type = IS_MX6SX,
 	.bch_max_ecc_strength = 62,
@@ -1010,6 +1016,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 {
 	struct gpmi_nand_data *this = chip->priv;
 	struct bch_geometry *nfc_geo = &this->bch_geometry;
+	void __iomem *bch_regs = this->resources.bch_regs;
 	void          *payload_virt;
 	dma_addr_t    payload_phys;
 	void          *auxiliary_virt;
@@ -1018,6 +1025,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	unsigned char *status;
 	unsigned int  max_bitflips = 0;
 	int           ret;
+	int flag = 0;
 
 	dev_dbg(this->dev, "page number is : %d\n", page);
 	ret = read_page_prepare(this, buf, nfc_geo->payload_size,
@@ -1050,9 +1058,16 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	status = auxiliary_virt + nfc_geo->auxiliary_status_offset;
 
 	for (i = 0; i < nfc_geo->ecc_chunk_count; i++, status++) {
-		if ((*status == STATUS_GOOD) || (*status == STATUS_ERASED))
+		if (*status == STATUS_GOOD)
 			continue;
 
+		if (*status == STATUS_ERASED) {
+			if (GPMI_IS_MX6QP(this) || GPMI_IS_MX7(this))
+				if (readl(bch_regs + HW_BCH_DEBUG1))
+					flag = 1;
+			continue;
+		}
+
 		if (*status == STATUS_UNCORRECTABLE) {
 			mtd->ecc_stats.failed++;
 			continue;
@@ -1081,6 +1096,10 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			nfc_geo->payload_size,
 			payload_virt, payload_phys);
 
+	/* if bitflip occurred in erased page, change data to all 0xff */
+	if (flag)
+		memset(buf, 0xff, nfc_geo->payload_size);
+
 	return max_bitflips;
 }
 
@@ -1991,6 +2010,9 @@ static const struct of_device_id gpmi_nand_id_table[] = {
 		.compatible = "fsl,imx6q-gpmi-nand",
 		.data = &gpmi_devdata_imx6q,
 	}, {
+		.compatible = "fsl,imx6qp-gpmi-nand",
+		.data = (void *)&gpmi_devdata_imx6qp,
+	}, {
 		.compatible = "fsl,imx6sx-gpmi-nand",
 		.data = &gpmi_devdata_imx6sx,
 	}, {
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
index 1e105f1..36ffeda 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
@@ -123,6 +123,7 @@ enum gpmi_type {
 	IS_MX23,
 	IS_MX28,
 	IS_MX6Q,
+	IS_MX6QP,
 	IS_MX6SX,
 	IS_MX7D
 };
@@ -306,9 +307,11 @@ void gpmi_copy_bits(u8 *dst, size_t dst_bit_off,
 #define GPMI_IS_MX23(x)		((x)->devdata->type == IS_MX23)
 #define GPMI_IS_MX28(x)		((x)->devdata->type == IS_MX28)
 #define GPMI_IS_MX6Q(x)		((x)->devdata->type == IS_MX6Q)
+#define GPMI_IS_MX6QP(x)	((x)->devdata->type == IS_MX6QP)
 #define GPMI_IS_MX6SX(x)	((x)->devdata->type == IS_MX6SX)
 #define GPMI_IS_MX7D(x)		((x)->devdata->type == IS_MX7D)
 
-#define GPMI_IS_MX6(x)		(GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x))
+#define GPMI_IS_MX6(x)		(GPMI_IS_MX6Q(x) || GPMI_IS_MX6QP(x)\
+	   || GPMI_IS_MX6SX(x))
 #define GPMI_IS_MX7(x)		(GPMI_IS_MX7D(x))
 #endif
-- 
1.9.1

Re: [PATCH 5/6] mtd: nand: gpmi: correct bitflip for erased NAND page

[Andrea Scian]

Dear Han Xu,


Il 28/07/2015 19:50, Han Xu ha scritto:
> i.MX6QP and i.MX7D BCH module integrated a new feature to detect the
> bitflip number for erased NAND page. So for these two platform, set the
> erase threshold to gf/2 and if bitflip detected, GPMI driver will
> correct the data to all 0xFF.


That's an iteresting feature of iMX6QP
We were discussing about erased NAND page bitflip in MTD ML a few days ago.

Is gf/2 a right value to choose? Why?
IIUC, in a previuos MTD-level patch [1] Brian used ECC strength as 
threshold.

Could you please also share with which NAND device you tested this patch?

Kind regards and TIA,

-- 

Andrea SCIAN

DAVE Embedded Systems

[1] http://article.gmane.org/gmane.linux.drivers.mtd/52216

Re: [PATCH 5/6] mtd: nand: gpmi: correct bitflip for erased NAND page

[Han Xu]

Hi Andrea,

The threshold gf/2 is referred to Huang Shijie's previous patch for bitflip,

http://lists.infradead.org/pipermail/linux-mtd/2014-January/051513.html

To verify the function, I raw write the whole NAND page with 0xFF and several
scattered bits with 0x0 to fake the bitflip, since the real bitflip is
unpredictable and tested the feature with Micron MT29F64G08AFAAA.


On Wed, Jul 29, 2015 at 3:05 AM, Andrea Scian <rnd4@dave-tech.it> wrote:
> Dear Han Xu,
>
>
> Il 28/07/2015 19:50, Han Xu ha scritto:
>>
>> i.MX6QP and i.MX7D BCH module integrated a new feature to detect the
>> bitflip number for erased NAND page. So for these two platform, set the
>> erase threshold to gf/2 and if bitflip detected, GPMI driver will
>> correct the data to all 0xFF.
>
>
>
> That's an iteresting feature of iMX6QP
> We were discussing about erased NAND page bitflip in MTD ML a few days ago.
>
> Is gf/2 a right value to choose? Why?
> IIUC, in a previuos MTD-level patch [1] Brian used ECC strength as
> threshold.
>
> Could you please also share with which NAND device you tested this patch?
>
> Kind regards and TIA,
>
> --
>
> Andrea SCIAN
>
> DAVE Embedded Systems
>
> [1] http://article.gmane.org/gmane.linux.drivers.mtd/52216
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 5/6] mtd: nand: gpmi: correct bitflip for erased NAND page

[Andrea Scian]

Il 29/07/2015 16:34, Han Xu ha scritto:
> Hi Andrea,
>
> The threshold gf/2 is referred to Huang Shijie's previous patch for bitflip,
>
> http://lists.infradead.org/pipermail/linux-mtd/2014-January/051513.html

Thanks for pointing out the reference.
Looking forward on the same thread, I saw that Brian already have some 
doubt about having the threshold correlated with gf instead of ecc_strength.

I think in this way (until someone, e.g. from micron, tell me something 
different ;-) ): erased pages act like the programmed one. They have 
bitflips and, unfortunately, cannot be protected by an ECC-like algorithm.
If, let's say, your NAND device need a 30 bit ECC protection over 1024 
byte page, this is nearly the same for an erased page.

As additional thought: what happens if you reports that an erased page 
has too much bitflips? UBIFS will fail badly [1]

Usually you never reach the "uncorrectable ECC error" level on standard 
situation (even on MLC ;-) ) because as soon as bitflips are more than a 
given threshold [2] those blocks are scrubbed and you're in the safe 
area again.
If you report ECC errors before this threshold, I think we fake the 
scrubbing functionality of UBI (which, yes, AFAIK should work on erased 
blocks too, why not?)

As first instance I would choose ECC strength as value to use, apart 
from the fact that I'm wondering what's happens if:
* my erased block is close to this value (let's say ECC strength -1)
* I write some data on it (with ECC)
* this write probably increase bitflips (only an erase operation will 
lower bitflip events) and, even worst, it will point me close to "ECC 
strength + 1" bitflips

> To verify the function, I raw write the whole NAND page with 0xFF and several
> scattered bits with 0x0 to fake the bitflip, since the real bitflip is
> unpredictable and tested the feature with Micron MT29F64G08AFAAA.

Ok thanks.

IIUC MT29F64G08AFAAA is an SLC NAND device but probably, due it's size, 
not a "real" SLC device and should have MLC like behavior.

I think you can easily trigger this situation (as I do) as follows:
* ubiformat, ubiattach, ubimkvol on a NAND MTD partition
* mount -t ubifs that volume
* get the physical address of LEB1 and LEB2 (somehow.. ;-) ). They have 
lots of erased space that UBIFS will check at boot time
* umount, ubidetach the partition
* do a nanddump lots of times (let's say from 10k to 100k) on those two PEBs
* sooner or later you'll see some bitflips on erased page
* try to mount UBIFS again: without patch it should fail, with your 
addition you should see that your erased-page check works correctly and 
UBIFS mounts successfully

Maybe I'm a bit OT regarding this patch, but I think this is an 
interesting point to discuss about.
Any comment is welcome

Kind Regards,

-- 

Andrea SCIAN

DAVE Embedded Systems

[1] http://lists.infradead.org/pipermail/linux-mtd/2015-July/060168.html
[2] http://lists.infradead.org/pipermail/linux-mtd/2015-January/057334.html