[PATCH v4 0/5] Micron SLC NAND filling block

[PATCH v4 0/5] Micron SLC NAND filling block

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

Hi,

on some legacy planar 2D Micron NAND devices when a block erase command is
issued, occasionally even though a block erase operation completes and returns
a pass status, the flash block may not be completely erased. Subsequent
operations to this block on very rare cases can result in subtle failures or
corruption. These extremely rare cases should nevertheless be considered. This
patchset is to address this potential issue.

After submission of patch V1 [1] and V2 [2], we stopped its update since we get
stuck in the solution on how to avoid the power-loss issue in case power-cut
hits the block filling. In the v1 and v2, to avoid this issue, we always damaged
page0, page1, this's based on the hypothesis that NAND FS is UBIFS. This
FS-specifical code is unacceptable in the MTD layer. Also, it cannot cover all
NAND based file system. Based on the current discussion, seems that re-write all
first 15 page from page0 is a satisfactory solution.

Meanwhile, I borrowed one idea from Miquel Raynal patchset [3], in which keeps
a recode of programmed pages, base on it, for most of the cases, we don't need
to read every page to see if current erasing block is a partially programmed
block.

Changelog:

v3 - v4:
    1. In the patch 4/5, change to directly use ecc.strength to judge the page
       is a empty page or not, rather than max_bitflips < mtd->bitflip_threshold
    2. In the patch 5/5, for the powerloss case, from the next time boot up,
       lots of page will be programmed from >page15 address, if still using
       first_p as GENMASK() bitmask starting position, writtenp will be always 0,
       fix it by changing its bitmask starting at bit position 0.

v2 - v3:
    1. Rebase patch to the latest MTD git tree
    2. Add a record that keeps tracking the programmed pages in the first 16
       pages
    3. Change from program odd pages, damage page 0 and page 1, to program all
       first 15 pages
    4. Address issues which exist in the V2.

v1 - v2:
    1. Rebased V1 to latest Linux kernel.
    2. Add erase preparation function pointer in nand_manufacturer_ops.


[1] https://www.spinics.net/lists/linux-mtd/msg04112.html
[2] https://www.spinics.net/lists/linux-mtd/msg04450.html
[3] https://www.spinics.net/lists/linux-mtd/msg13083.html


Bean Huo (5):
  mtd: rawnand: group all NAND specific ops into new nand_chip_ops
  mtd: rawnand: Add {pre,post}_erase hooks in nand_chip_ops
  mtd: rawnand: Add write_oob hook in nand_chip_ops
  mtd: rawnand: Introduce a new function nand_check_is_erased_page()
  mtd: rawnand: micron: Micron SLC NAND filling block

 drivers/mtd/nand/raw/internals.h     |   3 +-
 drivers/mtd/nand/raw/nand_base.c     |  88 +++++++++++++++++++----
 drivers/mtd/nand/raw/nand_hynix.c    |   2 +-
 drivers/mtd/nand/raw/nand_macronix.c |  10 +--
 drivers/mtd/nand/raw/nand_micron.c   | 104 ++++++++++++++++++++++++++-
 include/linux/mtd/rawnand.h          |  40 +++++++----
 6 files changed, 212 insertions(+), 35 deletions(-)

-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 1/5] mtd: rawnand: group all NAND specific ops into new nand_chip_ops

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

This patch is to create a new structure nand_chip_ops, and take all NAND
specific functions out from nand_chip and put them in this new structure.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/mtd/nand/raw/nand_base.c     | 20 +++++++++---------
 drivers/mtd/nand/raw/nand_hynix.c    |  2 +-
 drivers/mtd/nand/raw/nand_macronix.c | 10 ++++-----
 drivers/mtd/nand/raw/nand_micron.c   |  2 +-
 include/linux/mtd/rawnand.h          | 31 +++++++++++++++++-----------
 5 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 2d2a216af120..7af21cf49290 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -3234,10 +3234,10 @@ static int nand_setup_read_retry(struct nand_chip *chip, int retry_mode)
 	if (retry_mode >= chip->read_retries)
 		return -EINVAL;
 
-	if (!chip->setup_read_retry)
+	if (!chip->ops.setup_read_retry)
 		return -EOPNOTSUPP;
 
-	return chip->setup_read_retry(chip, retry_mode);
+	return chip->ops.setup_read_retry(chip, retry_mode);
 }
 
 static void nand_wait_readrdy(struct nand_chip *chip)
@@ -4481,8 +4481,8 @@ static int nand_suspend(struct mtd_info *mtd)
 	int ret = 0;
 
 	mutex_lock(&chip->lock);
-	if (chip->suspend)
-		ret = chip->suspend(chip);
+	if (chip->ops.suspend)
+		ret = chip->ops.suspend(chip);
 	if (!ret)
 		chip->suspended = 1;
 	mutex_unlock(&chip->lock);
@@ -4500,8 +4500,8 @@ static void nand_resume(struct mtd_info *mtd)
 
 	mutex_lock(&chip->lock);
 	if (chip->suspended) {
-		if (chip->resume)
-			chip->resume(chip);
+		if (chip->ops.resume)
+			chip->ops.resume(chip);
 		chip->suspended = 0;
 	} else {
 		pr_err("%s called for a chip which is not in suspended state\n",
@@ -4530,10 +4530,10 @@ static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
 
-	if (!chip->lock_area)
+	if (!chip->ops.lock_area)
 		return -ENOTSUPP;
 
-	return chip->lock_area(chip, ofs, len);
+	return chip->ops.lock_area(chip, ofs, len);
 }
 
 /**
@@ -4546,10 +4546,10 @@ static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
 
-	if (!chip->unlock_area)
+	if (!chip->ops.unlock_area)
 		return -ENOTSUPP;
 
-	return chip->unlock_area(chip, ofs, len);
+	return chip->ops.unlock_area(chip, ofs, len);
 }
 
 /* Set default functions */
diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c
index 7caedaa5b9e5..7d1be53f27f3 100644
--- a/drivers/mtd/nand/raw/nand_hynix.c
+++ b/drivers/mtd/nand/raw/nand_hynix.c
@@ -337,7 +337,7 @@ static int hynix_mlc_1xnm_rr_init(struct nand_chip *chip,
 	rr->nregs = nregs;
 	rr->regs = hynix_1xnm_mlc_read_retry_regs;
 	hynix->read_retry = rr;
-	chip->setup_read_retry = hynix_nand_setup_read_retry;
+	chip->ops.setup_read_retry = hynix_nand_setup_read_retry;
 	chip->read_retries = nmodes;
 
 out:
diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
index 09c254c97b5c..1472f925f386 100644
--- a/drivers/mtd/nand/raw/nand_macronix.c
+++ b/drivers/mtd/nand/raw/nand_macronix.c
@@ -130,7 +130,7 @@ static void macronix_nand_onfi_init(struct nand_chip *chip)
 		return;
 
 	chip->read_retries = MACRONIX_NUM_READ_RETRY_MODES;
-	chip->setup_read_retry = macronix_nand_setup_read_retry;
+	chip->ops.setup_read_retry = macronix_nand_setup_read_retry;
 
 	if (p->supports_set_get_features) {
 		bitmap_set(p->set_feature_list,
@@ -242,8 +242,8 @@ static void macronix_nand_block_protection_support(struct nand_chip *chip)
 	bitmap_set(chip->parameters.set_feature_list,
 		   ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
 
-	chip->lock_area = mxic_nand_lock;
-	chip->unlock_area = mxic_nand_unlock;
+	chip->ops.lock_area = mxic_nand_lock;
+	chip->ops.unlock_area = mxic_nand_unlock;
 }
 
 static int nand_power_down_op(struct nand_chip *chip)
@@ -312,8 +312,8 @@ static void macronix_nand_deep_power_down_support(struct nand_chip *chip)
 	if (i < 0)
 		return;
 
-	chip->suspend = mxic_nand_suspend;
-	chip->resume = mxic_nand_resume;
+	chip->ops.suspend = mxic_nand_suspend;
+	chip->ops.resume = mxic_nand_resume;
 }
 
 static int macronix_nand_init(struct nand_chip *chip)
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index b2b047b245f4..b3485b0995ad 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -84,7 +84,7 @@ static int micron_nand_onfi_init(struct nand_chip *chip)
 		struct nand_onfi_vendor_micron *micron = (void *)p->onfi->vendor;
 
 		chip->read_retries = micron->read_retry_options;
-		chip->setup_read_retry = micron_nand_setup_read_retry;
+		chip->ops.setup_read_retry = micron_nand_setup_read_retry;
 	}
 
 	if (p->supports_set_get_features) {
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 0f45b6984ad1..62932cc3ed8d 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1025,6 +1025,23 @@ struct nand_legacy {
 	struct nand_controller dummy_controller;
 };
 
+/**
+ * struct nand_chip_ops - NAND Chip specific operations
+ * @suspend:           [REPLACEABLE] specific NAND device suspend operation
+ * @resume:            [REPLACEABLE] specific NAND device resume operation
+ * @lock_area:         [REPLACEABLE] specific NAND chip lock operation
+ * @unlock_area:       [REPLACEABLE] specific NAND chip unlock operation
+ * @setup_read_retry:  [FLASHSPECIFIC] flash (vendor) specific function for
+ *                     setting the read-retry mode. Mostly needed for MLC NAND.
+ */
+struct nand_chip_ops {
+	int (*suspend)(struct nand_chip *chip);
+	void (*resume)(struct nand_chip *chip);
+	int (*lock_area)(struct nand_chip *chip, loff_t ofs, u64 len);
+	int (*unlock_area)(struct nand_chip *chip, loff_t ofs, u64 len);
+	int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
+};
+
 /**
  * struct nand_chip - NAND Private Flash Chip Data
  * @base:		Inherit from the generic NAND device
@@ -1033,8 +1050,6 @@ struct nand_legacy {
  *			you're modifying an existing driver that is using those
  *			fields/hooks, you should consider reworking the driver
  *			avoid using them.
- * @setup_read_retry:	[FLASHSPECIFIC] flash (vendor) specific function for
- *			setting the read-retry mode. Mostly needed for MLC NAND.
  * @ecc:		[BOARDSPECIFIC] ECC control structure
  * @buf_align:		minimum buffer alignment required by a platform
  * @oob_poi:		"poison value buffer," used for laying out OOB data
@@ -1079,8 +1094,6 @@ struct nand_legacy {
  * @lock:		lock protecting the suspended field. Also used to
  *			serialize accesses to the NAND device.
  * @suspended:		set to 1 when the device is suspended, 0 when it's not.
- * @suspend:		[REPLACEABLE] specific NAND device suspend operation
- * @resume:		[REPLACEABLE] specific NAND device resume operation
  * @bbt:		[INTERN] bad block table pointer
  * @bbt_td:		[REPLACEABLE] bad block table descriptor for flash
  *			lookup.
@@ -1094,8 +1107,7 @@ struct nand_legacy {
  * @manufacturer:	[INTERN] Contains manufacturer information
  * @manufacturer.desc:	[INTERN] Contains manufacturer's description
  * @manufacturer.priv:	[INTERN] Contains manufacturer private information
- * @lock_area:		[REPLACEABLE] specific NAND chip lock operation
- * @unlock_area:	[REPLACEABLE] specific NAND chip unlock operation
+ * @ops:		NAND-specific operations description structure
  */
 
 struct nand_chip {
@@ -1103,8 +1115,6 @@ struct nand_chip {
 
 	struct nand_legacy legacy;
 
-	int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
-
 	unsigned int options;
 	unsigned int bbt_options;
 
@@ -1136,8 +1146,6 @@ struct nand_chip {
 
 	struct mutex lock;
 	unsigned int suspended : 1;
-	int (*suspend)(struct nand_chip *chip);
-	void (*resume)(struct nand_chip *chip);
 
 	uint8_t *oob_poi;
 	struct nand_controller *controller;
@@ -1158,8 +1166,7 @@ struct nand_chip {
 		void *priv;
 	} manufacturer;
 
-	int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
-	int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
+	struct nand_chip_ops ops;
 };
 
 extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 2/5] mtd: rawnand: Add {pre, post}_erase hooks in nand_chip_ops

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

Add {pre,post}_erase hooks in the structure nand_chip_ops:
pre_erase will be called before a block is physically erased.
post_erase will be called after a block is erased.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/mtd/nand/raw/nand_base.c | 18 +++++++++++++-----
 include/linux/mtd/rawnand.h      | 16 ++++++++++------
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 7af21cf49290..e90b7ae878e2 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4318,7 +4318,7 @@ static int nand_erase(struct mtd_info *mtd, struct erase_info *instr)
 int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr,
 		    int allowbbt)
 {
-	int page, pages_per_block, ret, chipnr;
+	int page, pages_per_block, ret, chipnr, eb;
 	loff_t len;
 
 	pr_debug("%s: start = 0x%012llx, len = %llu\n",
@@ -4372,16 +4372,24 @@ int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr,
 		    (page + pages_per_block))
 			chip->pagecache.page = -1;
 
-		ret = nand_erase_op(chip, (page & chip->pagemask) >>
-				    (chip->phys_erase_shift - chip->page_shift));
+		eb = (page & chip->pagemask) >>
+			(chip->phys_erase_shift - chip->page_shift);
+
+		if (chip->ops.pre_erase)
+			chip->ops.pre_erase(chip, eb);
+
+		ret = nand_erase_op(chip, eb);
 		if (ret) {
-			pr_debug("%s: failed erase, page 0x%08x\n",
-					__func__, page);
+			pr_debug("%s: failed erase block %d, page 0x%08x\n",
+					__func__, eb, page);
 			instr->fail_addr =
 				((loff_t)page << chip->page_shift);
 			goto erase_exit;
 		}
 
+		if (chip->ops.post_erase)
+			chip->ops.post_erase(chip, eb);
+
 		/* Increment page address and decrement length */
 		len -= (1ULL << chip->phys_erase_shift);
 		page += pages_per_block;
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 62932cc3ed8d..df3d4b3ef2f6 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1027,12 +1027,14 @@ struct nand_legacy {
 
 /**
  * struct nand_chip_ops - NAND Chip specific operations
- * @suspend:           [REPLACEABLE] specific NAND device suspend operation
- * @resume:            [REPLACEABLE] specific NAND device resume operation
- * @lock_area:         [REPLACEABLE] specific NAND chip lock operation
- * @unlock_area:       [REPLACEABLE] specific NAND chip unlock operation
- * @setup_read_retry:  [FLASHSPECIFIC] flash (vendor) specific function for
- *                     setting the read-retry mode. Mostly needed for MLC NAND.
+ * @suspend:		[REPLACEABLE] specific NAND device suspend operation
+ * @resume:		[REPLACEABLE] specific NAND device resume operation
+ * @lock_area:		[REPLACEABLE] specific NAND chip lock operation
+ * @unlock_area:	[REPLACEABLE] specific NAND chip unlock operation
+ * @setup_read_retry:	[FLASHSPECIFIC] flash (vendor) specific function for
+ *			setting the read-retry mode. Mostly needed for MLC NAND.
+ * @pre_erase:		[FLASHSPECIFIC] prepare a physical erase block
+ * @post_erase:		[FLASHSPECIFIC] physical block erase post
  */
 struct nand_chip_ops {
 	int (*suspend)(struct nand_chip *chip);
@@ -1040,6 +1042,8 @@ struct nand_chip_ops {
 	int (*lock_area)(struct nand_chip *chip, loff_t ofs, u64 len);
 	int (*unlock_area)(struct nand_chip *chip, loff_t ofs, u64 len);
 	int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
+	int (*pre_erase)(struct nand_chip *chip, u32 eraseblock);
+	int (*post_erase)(struct nand_chip *chip, u32 eraseblock);
 };
 
 /**
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 3/5] mtd: rawnand: Add write_oob hook in nand_chip_ops

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

Break the function nand_write_oob() into two functions, and one of them
is named nand_write_oob_nand(), which will be assigned to new added hook
write_oob by default. The hook write_oob will be overwritten in the NAND
vendor lower-level driver if needed.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/mtd/nand/raw/internals.h | 3 ++-
 drivers/mtd/nand/raw/nand_base.c | 9 +++++++++
 include/linux/mtd/rawnand.h      | 3 +++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
index 03866b0aadea..94d300a207ac 100644
--- a/drivers/mtd/nand/raw/internals.h
+++ b/drivers/mtd/nand/raw/internals.h
@@ -99,7 +99,8 @@ int nand_read_param_page_op(struct nand_chip *chip, u8 page, void *buf,
 void nand_decode_ext_id(struct nand_chip *chip);
 void panic_nand_wait(struct nand_chip *chip, unsigned long timeo);
 void sanitize_string(uint8_t *s, size_t len);
-
+int nand_write_oob_nand(struct nand_chip *chip, loff_t to,
+			 struct mtd_oob_ops *ops);
 static inline bool nand_has_exec_op(struct nand_chip *chip)
 {
 	if (!chip->controller || !chip->controller->ops ||
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index e90b7ae878e2..09ee490c08a9 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4267,6 +4267,13 @@ static int nand_write_oob(struct mtd_info *mtd, loff_t to,
 			  struct mtd_oob_ops *ops)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
+
+	return chip->ops.write_oob(chip, to, ops);
+}
+
+int nand_write_oob_nand(struct nand_chip *chip, loff_t to,
+			struct mtd_oob_ops *ops)
+{
 	int ret;
 
 	ops->retlen = 0;
@@ -4573,6 +4580,8 @@ static void nand_set_defaults(struct nand_chip *chip)
 
 	if (!chip->buf_align)
 		chip->buf_align = 1;
+
+	chip->ops.write_oob = nand_write_oob_nand;
 }
 
 /* Sanitize ONFI strings so we can safely print them */
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index df3d4b3ef2f6..3d75e50e5b75 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1035,6 +1035,7 @@ struct nand_legacy {
  *			setting the read-retry mode. Mostly needed for MLC NAND.
  * @pre_erase:		[FLASHSPECIFIC] prepare a physical erase block
  * @post_erase:		[FLASHSPECIFIC] physical block erase post
+ * @write_oob:		[REPLACEABLE] Raw NAND write operation
  */
 struct nand_chip_ops {
 	int (*suspend)(struct nand_chip *chip);
@@ -1044,6 +1045,8 @@ struct nand_chip_ops {
 	int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
 	int (*pre_erase)(struct nand_chip *chip, u32 eraseblock);
 	int (*post_erase)(struct nand_chip *chip, u32 eraseblock);
+	int (*write_oob)(struct nand_chip *chip, loff_t to,
+			 struct mtd_oob_ops *ops);
 };
 
 /**
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 4/5] mtd: rawnand: Introduce a new function nand_check_is_erased_page()

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

Add a new function nand_check_is_erased_page() in nand_base.c, which is
used to check whether one programmable page is empty or already programmed.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/mtd/nand/raw/nand_base.c | 41 ++++++++++++++++++++++++++++++++
 include/linux/mtd/rawnand.h      |  2 ++
 2 files changed, 43 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 09ee490c08a9..2b9862d9979b 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -2646,6 +2646,47 @@ int nand_check_erased_ecc_chunk(void *data, int datalen,
 }
 EXPORT_SYMBOL(nand_check_erased_ecc_chunk);
 
+/**
+ * nand_check_is_erased_page - check if this page is a empty page
+ * @chip: nand chip info structure
+ * @page_data: data buffer containing the data in the page being checked
+ * @oob: indicate if chip->oob_poi points to oob date of the page
+ *
+ * Returns true if this is an un-programmed page, false otherwise.
+ */
+int nand_check_is_erased_page(struct nand_chip *chip, u8 *page_data, bool oob)
+{
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	int ret, i;
+	u8 *databuf, *eccbuf = NULL;
+	struct mtd_oob_region oobregion;
+	int datasize, eccbytes = 0;
+
+	databuf = page_data;
+	datasize = chip->ecc.size;
+
+	if (oob) {
+		mtd_ooblayout_ecc(mtd, 0, &oobregion);
+		eccbuf = chip->oob_poi + oobregion.offset;
+		eccbytes = chip->ecc.bytes;
+	}
+	max_bitflips = 0;
+
+	for (i = 0; i < chip->ecc.steps; i++) {
+		ret = nand_check_erased_ecc_chunk(databuf, datasize,
+						  eccbuf, eccbytes,
+						  NULL, 0, chip->ecc.strength);
+		if (ret < 0)
+			return false;
+
+		databuf += chip->ecc.size;
+		eccbuf += chip->ecc.bytes;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(nand_check_is_erased_page);
+
 /**
  * nand_read_page_raw_notsupp - dummy read raw page function
  * @chip: nand chip info structure
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 3d75e50e5b75..718ce81eb111 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1337,6 +1337,8 @@ int nand_check_erased_ecc_chunk(void *data, int datalen,
 				void *extraoob, int extraooblen,
 				int threshold);
 
+int nand_check_is_erased_page(struct nand_chip *chip, u8 *page_data, bool oob);
+
 int nand_ecc_choose_conf(struct nand_chip *chip,
 			 const struct nand_ecc_caps *caps, int oobavail);
 
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 5/5] mtd: rawnand: micron: Micron SLC NAND filling block

[Bean Huo]

From: Bean Huo <beanhuo@micron.com>

On some legacy planar 2D Micron NAND devices when a block erase command
is issued, occasionally even though a block erase operation completes and
returns a pass status, the flash block may not be completely erased.
Subsequent operations to this block on very rare cases can result in subtle
failures or corruption. These extremely rare cases should nevertheless be
considered. These rare occurrences have been observed on partially written
blocks.

To avoid this rare occurrence, we should make sure that at least 15 pages
have been programmed to a block before it is erased. In case we find that
less than 15 pages have been programmed, we will rewrite first 15 pages of
block.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/mtd/nand/raw/nand_micron.c | 102 +++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index b3485b0995ad..c5fd9e60f46d 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -36,6 +36,9 @@
 #define NAND_ECC_STATUS_1_3_CORRECTED	BIT(4)
 #define NAND_ECC_STATUS_7_8_CORRECTED	(BIT(4) | BIT(3))
 
+#define MICRON_SHALLOW_ERASE_MIN_PAGE 15
+#define MICRON_PAGE_MASK_TRIGGER GENMASK(MICRON_SHALLOW_ERASE_MIN_PAGE, 0)
+
 struct nand_onfi_vendor_micron {
 	u8 two_plane_read;
 	u8 read_cache;
@@ -64,6 +67,7 @@ struct micron_on_die_ecc {
 
 struct micron_nand {
 	struct micron_on_die_ecc ecc;
+	u16 *writtenp;
 };
 
 static int micron_nand_setup_read_retry(struct nand_chip *chip, int retry_mode)
@@ -429,6 +433,93 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
 	return MICRON_ON_DIE_SUPPORTED;
 }
 
+static int micron_nand_pre_erase(struct nand_chip *chip, u32 eraseblock)
+{
+	struct micron_nand *micron = nand_get_manufacturer_data(chip);
+	struct mtd_info *mtd = nand_to_mtd(chip);
+	u8 last_page = MICRON_SHALLOW_ERASE_MIN_PAGE - 1;
+	u32 page;
+	u8 *data_buf;
+	int ret, i;
+
+	data_buf = nand_get_data_buf(chip);
+	WARN_ON(!data_buf);
+
+	if (likely(micron->writtenp[eraseblock] & BIT(last_page)))
+		return 0;
+
+	page = eraseblock << (chip->phys_erase_shift - chip->page_shift);
+
+	if (unlikely(micron->writtenp[eraseblock] == 0)) {
+		ret = nand_read_page_raw(chip, data_buf, 1, page + last_page);
+		if (ret)
+			return ret; /* Read error */
+		ret = nand_check_is_erased_page(chip, data_buf, true);
+		if (!ret)
+			return 0;
+	}
+
+	memset(data_buf, 0x00, mtd->writesize);
+
+	for (i = 0; i < MICRON_SHALLOW_ERASE_MIN_PAGE; i++) {
+		ret = nand_write_page_raw(chip, data_buf, false, page + i);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int micron_nand_post_erase(struct nand_chip *chip, u32 eraseblock)
+{
+	struct micron_nand *micron = nand_get_manufacturer_data(chip);
+
+	if (!micron)
+		return -EINVAL;
+
+	micron->writtenp[eraseblock] = 0;
+
+	return 0;
+}
+
+static int micron_nand_write_oob(struct nand_chip *chip, loff_t to,
+				 struct mtd_oob_ops *ops)
+{
+	struct micron_nand *micron = nand_get_manufacturer_data(chip);
+	u32 eb_sz = nanddev_eraseblock_size(&chip->base);
+	u32 p_sz = nanddev_page_size(&chip->base);
+	u32 ppeb = nanddev_pages_per_eraseblock(&chip->base);
+	u32 nb_p_tot = ops->len / p_sz;
+	u32 first_eb = DIV_ROUND_DOWN_ULL(to, eb_sz);
+	u32 first_p = DIV_ROUND_UP_ULL(to - (first_eb * eb_sz), p_sz);
+	u32 nb_eb = DIV_ROUND_UP_ULL(first_p + nb_p_tot, ppeb);
+	u32 remaining_p, eb, nb_p;
+	int ret;
+
+	ret = nand_write_oob_nand(chip, to, ops);
+
+	if (ret || ops->len != ops->retlen)
+		return ret;
+
+	/* Mark the last pages of the first erase block to write */
+	nb_p = min(nb_p_tot, ppeb - first_p);
+	micron->writtenp[first_eb] |= GENMASK(first_p + nb_p, 0) &
+					MICRON_PAGE_MASK_TRIGGER;
+	remaining_p = nb_p_tot - nb_p;
+
+	/* Mark all the pages of all "in-the-middle" erase blocks */
+	for (eb = first_eb + 1; eb < first_eb + nb_eb - 1; eb++) {
+		micron->writtenp[eb] |= MICRON_PAGE_MASK_TRIGGER;
+		remaining_p -= ppeb;
+	}
+
+	/* Mark the first pages of the last erase block to write */
+	if (remaining_p)
+		micron->writtenp[eb] |= GENMASK(remaining_p - 1, 0) &
+					MICRON_PAGE_MASK_TRIGGER;
+		return 0;
+}
+
 static int micron_nand_init(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
@@ -515,6 +606,17 @@ static int micron_nand_init(struct nand_chip *chip)
 		}
 	}
 
+	if (nand_is_slc(chip)) {
+		micron->writtenp = kcalloc(nanddev_neraseblocks(&chip->base),
+					   sizeof(u16), GFP_KERNEL);
+		if (!micron->writtenp)
+			goto err_free_manuf_data;
+
+		chip->ops.write_oob = micron_nand_write_oob;
+		chip->ops.pre_erase = micron_nand_pre_erase;
+		chip->ops.post_erase = micron_nand_post_erase;
+	}
+
 	return 0;
 
 err_free_manuf_data:
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 4/5] mtd: rawnand: Introduce a new function nand_check_is_erased_page()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3044 bytes --]

Hi Bean,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.7-rc6 next-20200518]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Bean-Huo/Micron-SLC-NAND-filling-block/20200518-220231
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git b9bbe6ed63b2b9f2c9ee5cbd0f2c946a2723f4ce
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

drivers/mtd/nand/raw/nand_base.c: In function 'nand_check_is_erased_page':
>> drivers/mtd/nand/raw/nand_base.c:2611:2: error: 'max_bitflips' undeclared (first use in this function)
2611 |  max_bitflips = 0;
|  ^~~~~~~~~~~~
drivers/mtd/nand/raw/nand_base.c:2611:2: note: each undeclared identifier is reported only once for each function it appears in

vim +/max_bitflips +2611 drivers/mtd/nand/raw/nand_base.c

  2586	
  2587	/**
  2588	 * nand_check_is_erased_page - check if this page is a empty page
  2589	 * @chip: nand chip info structure
  2590	 * @page_data: data buffer containing the data in the page being checked
  2591	 * @oob: indicate if chip->oob_poi points to oob date of the page
  2592	 *
  2593	 * Returns true if this is an un-programmed page, false otherwise.
  2594	 */
  2595	int nand_check_is_erased_page(struct nand_chip *chip, u8 *page_data, bool oob)
  2596	{
  2597		struct mtd_info *mtd = nand_to_mtd(chip);
  2598		int ret, i;
  2599		u8 *databuf, *eccbuf = NULL;
  2600		struct mtd_oob_region oobregion;
  2601		int datasize, eccbytes = 0;
  2602	
  2603		databuf = page_data;
  2604		datasize = chip->ecc.size;
  2605	
  2606		if (oob) {
  2607			mtd_ooblayout_ecc(mtd, 0, &oobregion);
  2608			eccbuf = chip->oob_poi + oobregion.offset;
  2609			eccbytes = chip->ecc.bytes;
  2610		}
> 2611		max_bitflips = 0;
  2612	
  2613		for (i = 0; i < chip->ecc.steps; i++) {
  2614			ret = nand_check_erased_ecc_chunk(databuf, datasize,
  2615							  eccbuf, eccbytes,
  2616							  NULL, 0, chip->ecc.strength);
  2617			if (ret < 0)
  2618				return false;
  2619	
  2620			databuf += chip->ecc.size;
  2621			eccbuf += chip->ecc.bytes;
  2622		}
  2623	
  2624		return ret;
  2625	}
  2626	EXPORT_SYMBOL(nand_check_is_erased_page);
  2627	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53985 bytes --]

[-- Attachment #3: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 0/5] Micron SLC NAND filling block

[Miquel Raynal]

Hi Bean,

Bean Huo <huobean@gmail.com> wrote on Mon, 18 May 2020 15:59:38 +0200:

> From: Bean Huo <beanhuo@micron.com>
> 
> Hi,
> 
> on some legacy planar 2D Micron NAND devices when a block erase command is
> issued, occasionally even though a block erase operation completes and returns
> a pass status, the flash block may not be completely erased. Subsequent
> operations to this block on very rare cases can result in subtle failures or
> corruption. These extremely rare cases should nevertheless be considered. This
> patchset is to address this potential issue.
> 
> After submission of patch V1 [1] and V2 [2], we stopped its update since we get
> stuck in the solution on how to avoid the power-loss issue in case power-cut
> hits the block filling. In the v1 and v2, to avoid this issue, we always damaged
> page0, page1, this's based on the hypothesis that NAND FS is UBIFS. This
> FS-specifical code is unacceptable in the MTD layer. Also, it cannot cover all
> NAND based file system. Based on the current discussion, seems that re-write all
> first 15 page from page0 is a satisfactory solution.

We have a layering problem now. Maybe we should just have an MTD
internal variable like min_written_pages_before_erase that the Micron
driver could set to a !0 value.

Then, the handling could be done by the user (UBI/UBIFS, JFFS2, MTD
user if exported).

> 
> Meanwhile, I borrowed one idea from Miquel Raynal patchset [3], in which keeps
> a recode of programmed pages, base on it, for most of the cases, we don't need
> to read every page to see if current erasing block is a partially programmed
> block.
> 
> Changelog:
> 
> v3 - v4:
>     1. In the patch 4/5, change to directly use ecc.strength to judge the page
>        is a empty page or not, rather than max_bitflips < mtd->bitflip_threshold
>     2. In the patch 5/5, for the powerloss case, from the next time boot up,
>        lots of page will be programmed from >page15 address, if still using
>        first_p as GENMASK() bitmask starting position, writtenp will be always 0,
>        fix it by changing its bitmask starting at bit position 0.
> 
> v2 - v3:
>     1. Rebase patch to the latest MTD git tree
>     2. Add a record that keeps tracking the programmed pages in the first 16
>        pages
>     3. Change from program odd pages, damage page 0 and page 1, to program all
>        first 15 pages
>     4. Address issues which exist in the V2.
> 
> v1 - v2:
>     1. Rebased V1 to latest Linux kernel.
>     2. Add erase preparation function pointer in nand_manufacturer_ops.
> 
> 
> [1] https://www.spinics.net/lists/linux-mtd/msg04112.html
> [2] https://www.spinics.net/lists/linux-mtd/msg04450.html
> [3] https://www.spinics.net/lists/linux-mtd/msg13083.html
> 
> 
> Bean Huo (5):
>   mtd: rawnand: group all NAND specific ops into new nand_chip_ops
>   mtd: rawnand: Add {pre,post}_erase hooks in nand_chip_ops
>   mtd: rawnand: Add write_oob hook in nand_chip_ops
>   mtd: rawnand: Introduce a new function nand_check_is_erased_page()
>   mtd: rawnand: micron: Micron SLC NAND filling block

When you take my patches in your series, especially when not touching
them at all, you should keep my Authorship and SoB first, then add your
SoB.


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 5/5] mtd: rawnand: micron: Micron SLC NAND filling block

[Steve deRosier]

On Mon, May 18, 2020 at 7:00 AM Bean Huo <huobean@gmail.com> wrote:
>
> From: Bean Huo <beanhuo@micron.com>
>
> On some legacy planar 2D Micron NAND devices when a block erase command

I object the use of the qualifications you're putting in this
sentence. By saying "some legacy...." you're implying that there's a
set that does and a set that doesn't require this. Which then leads
the reader of this commit message to #1 look for which ones this
applies to vs not, and #2 want to remove/exclude the feature when
they're using a "current" device. The wiggle-word wording is confusing
and dishonest.

I've followed this discussion now intently and it seems like Micron is
either unable or unwilling to determine which specific devices this
does or doesn't apply to. If you are unable to identify and restrict
this functionality to a specific subset of devices, then the fact is
it's "all."  Let's just say that and eliminate the confusion. And
please also update your datasheets to indicate that this is the
correct algorithm for working with these devices. Better would be to
issue an errata on the chips and notify your customers. I feel for
those customers who aren't using Linux and don't know the reliability
problem they've been tracking down for the last couple of years is
already known but they don't have any way of knowing about it.

In your commit message, rewording to "On planar 2D Micron NAND devices
when a block erase command..." is sufficient.

- Steve


> is issued, occasionally even though a block erase operation completes and
> returns a pass status, the flash block may not be completely erased.
> Subsequent operations to this block on very rare cases can result in subtle
> failures or corruption. These extremely rare cases should nevertheless be
> considered. These rare occurrences have been observed on partially written
> blocks.
>
> To avoid this rare occurrence, we should make sure that at least 15 pages
> have been programmed to a block before it is erased. In case we find that
> less than 15 pages have been programmed, we will rewrite first 15 pages of
> block.
>
> Signed-off-by: Bean Huo <beanhuo@micron.com>
> ---
>  drivers/mtd/nand/raw/nand_micron.c | 102 +++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index b3485b0995ad..c5fd9e60f46d 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -36,6 +36,9 @@
>  #define NAND_ECC_STATUS_1_3_CORRECTED  BIT(4)
>  #define NAND_ECC_STATUS_7_8_CORRECTED  (BIT(4) | BIT(3))
>
> +#define MICRON_SHALLOW_ERASE_MIN_PAGE 15
> +#define MICRON_PAGE_MASK_TRIGGER GENMASK(MICRON_SHALLOW_ERASE_MIN_PAGE, 0)
> +
>  struct nand_onfi_vendor_micron {
>         u8 two_plane_read;
>         u8 read_cache;
> @@ -64,6 +67,7 @@ struct micron_on_die_ecc {
>
>  struct micron_nand {
>         struct micron_on_die_ecc ecc;
> +       u16 *writtenp;
>  };
>
>  static int micron_nand_setup_read_retry(struct nand_chip *chip, int retry_mode)
> @@ -429,6 +433,93 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip)
>         return MICRON_ON_DIE_SUPPORTED;
>  }
>
> +static int micron_nand_pre_erase(struct nand_chip *chip, u32 eraseblock)
> +{
> +       struct micron_nand *micron = nand_get_manufacturer_data(chip);
> +       struct mtd_info *mtd = nand_to_mtd(chip);
> +       u8 last_page = MICRON_SHALLOW_ERASE_MIN_PAGE - 1;
> +       u32 page;
> +       u8 *data_buf;
> +       int ret, i;
> +
> +       data_buf = nand_get_data_buf(chip);
> +       WARN_ON(!data_buf);
> +
> +       if (likely(micron->writtenp[eraseblock] & BIT(last_page)))
> +               return 0;
> +
> +       page = eraseblock << (chip->phys_erase_shift - chip->page_shift);
> +
> +       if (unlikely(micron->writtenp[eraseblock] == 0)) {
> +               ret = nand_read_page_raw(chip, data_buf, 1, page + last_page);
> +               if (ret)
> +                       return ret; /* Read error */
> +               ret = nand_check_is_erased_page(chip, data_buf, true);
> +               if (!ret)
> +                       return 0;
> +       }
> +
> +       memset(data_buf, 0x00, mtd->writesize);
> +
> +       for (i = 0; i < MICRON_SHALLOW_ERASE_MIN_PAGE; i++) {
> +               ret = nand_write_page_raw(chip, data_buf, false, page + i);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static int micron_nand_post_erase(struct nand_chip *chip, u32 eraseblock)
> +{
> +       struct micron_nand *micron = nand_get_manufacturer_data(chip);
> +
> +       if (!micron)
> +               return -EINVAL;
> +
> +       micron->writtenp[eraseblock] = 0;
> +
> +       return 0;
> +}
> +
> +static int micron_nand_write_oob(struct nand_chip *chip, loff_t to,
> +                                struct mtd_oob_ops *ops)
> +{
> +       struct micron_nand *micron = nand_get_manufacturer_data(chip);
> +       u32 eb_sz = nanddev_eraseblock_size(&chip->base);
> +       u32 p_sz = nanddev_page_size(&chip->base);
> +       u32 ppeb = nanddev_pages_per_eraseblock(&chip->base);
> +       u32 nb_p_tot = ops->len / p_sz;
> +       u32 first_eb = DIV_ROUND_DOWN_ULL(to, eb_sz);
> +       u32 first_p = DIV_ROUND_UP_ULL(to - (first_eb * eb_sz), p_sz);
> +       u32 nb_eb = DIV_ROUND_UP_ULL(first_p + nb_p_tot, ppeb);
> +       u32 remaining_p, eb, nb_p;
> +       int ret;
> +
> +       ret = nand_write_oob_nand(chip, to, ops);
> +
> +       if (ret || ops->len != ops->retlen)
> +               return ret;
> +
> +       /* Mark the last pages of the first erase block to write */
> +       nb_p = min(nb_p_tot, ppeb - first_p);
> +       micron->writtenp[first_eb] |= GENMASK(first_p + nb_p, 0) &
> +                                       MICRON_PAGE_MASK_TRIGGER;
> +       remaining_p = nb_p_tot - nb_p;
> +
> +       /* Mark all the pages of all "in-the-middle" erase blocks */
> +       for (eb = first_eb + 1; eb < first_eb + nb_eb - 1; eb++) {
> +               micron->writtenp[eb] |= MICRON_PAGE_MASK_TRIGGER;
> +               remaining_p -= ppeb;
> +       }
> +
> +       /* Mark the first pages of the last erase block to write */
> +       if (remaining_p)
> +               micron->writtenp[eb] |= GENMASK(remaining_p - 1, 0) &
> +                                       MICRON_PAGE_MASK_TRIGGER;
> +               return 0;
> +}
> +
>  static int micron_nand_init(struct nand_chip *chip)
>  {
>         struct mtd_info *mtd = nand_to_mtd(chip);
> @@ -515,6 +606,17 @@ static int micron_nand_init(struct nand_chip *chip)
>                 }
>         }
>
> +       if (nand_is_slc(chip)) {
> +               micron->writtenp = kcalloc(nanddev_neraseblocks(&chip->base),
> +                                          sizeof(u16), GFP_KERNEL);
> +               if (!micron->writtenp)
> +                       goto err_free_manuf_data;
> +
> +               chip->ops.write_oob = micron_nand_write_oob;
> +               chip->ops.pre_erase = micron_nand_pre_erase;
> +               chip->ops.post_erase = micron_nand_post_erase;
> +       }
> +
>         return 0;
>
>  err_free_manuf_data:
> --
> 2.17.1
>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 0/5] Micron SLC NAND filling block

[Bean Huo]

hi,  Miquel

On Mon, 2020-05-18 at 17:22 +0200, Miquel Raynal wrote:
> Hi Bean,
> 
> Bean Huo <huobean@gmail.com> wrote on Mon, 18 May 2020 15:59:38
> +0200:
> 
> > From: Bean Huo <beanhuo@micron.com>
> > 
> > After submission of patch V1 [1] and V2 [2], we stopped its update
> > since we get
> > stuck in the solution on how to avoid the power-loss issue in case
> > power-cut
> > hits the block filling. In the v1 and v2, to avoid this issue, we
> > always damaged
> > page0, page1, this's based on the hypothesis that NAND FS is UBIFS.
> > This
> > FS-specifical code is unacceptable in the MTD layer. Also, it
> > cannot cover all
> > NAND based file system. Based on the current discussion, seems that
> > re-write all
> > first 15 page from page0 is a satisfactory solution.
> 
> We have a layering problem now. Maybe we should just have an MTD
> internal variable like min_written_pages_before_erase that the Micron
> driver could set to a !0 value.
> 
> Then, the handling could be done by the user (UBI/UBIFS, JFFS2, MTD
> user if exported).
> 

This is NAND its own problem, if no significant adantage, I don't think
it's a good solution to extend the problem to the upper FS layer.
also, in the FS erase path, doesn't have the programmed pages counter.
we should repeat the same approach as we did in MTD layer.

> > 
> > Meanwhile, I borrowed one idea from Miquel Raynal patchset [3], in
> > which keeps
> > a recode of programmed pages, base on it, for most of the cases, we
> > don't need
> > to read every page to see if current erasing block is a partially
> > programmed
> > block.
> > 
> > Changelog:
> > 
> > v3 - v4:
> >     1. In the patch 4/5, change to directly use ecc.strength to
> > judge the page
> >        is a empty page or not, rather than max_bitflips < mtd-
> > >bitflip_threshold
> >     2. In the patch 5/5, for the powerloss case, from the next time
> > boot up,
> >        lots of page will be programmed from >page15 address, if
> > still using
> >        first_p as GENMASK() bitmask starting position, writtenp
> > will be always 0,
> >        fix it by changing its bitmask starting at bit position 0.
> > 
> > v2 - v3:
> >     1. Rebase patch to the latest MTD git tree
> >     2. Add a record that keeps tracking the programmed pages in the
> > first 16
> >        pages
> >     3. Change from program odd pages, damage page 0 and page 1, to
> > program all
> >        first 15 pages
> >     4. Address issues which exist in the V2.
> > 
> > v1 - v2:
> >     1. Rebased V1 to latest Linux kernel.
> >     2. Add erase preparation function pointer in
> > nand_manufacturer_ops.
> > 
> > 
> > [1] https://www.spinics.net/lists/linux-mtd/msg04112.html
> > [2] https://www.spinics.net/lists/linux-mtd/msg04450.html
> > [3] https://www.spinics.net/lists/linux-mtd/msg13083.html
> > 
> > 
> > Bean Huo (5):
> >   mtd: rawnand: group all NAND specific ops into new nand_chip_ops
> >   mtd: rawnand: Add {pre,post}_erase hooks in nand_chip_ops
> >   mtd: rawnand: Add write_oob hook in nand_chip_ops
> >   mtd: rawnand: Introduce a new function
> > nand_check_is_erased_page()
> >   mtd: rawnand: micron: Micron SLC NAND filling block
> 
> When you take my patches in your series, especially when not touching
> them at all, you should keep my Authorship and SoB first, then add
> your
> SoB.
> 

sorry for my fault, I thought adding your Signed-off-by in 3/5 is
suffient. you mean I should add your signed-off-by in 5/5 as well?
I will do that in next version.

thanks Miquel.


BTW: would you please help me review other code?


 
Bean


> 
> Thanks,
> Miquèl


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 0/5] Micron SLC NAND filling block

[Miquel Raynal]

Hi Bean,

Bean Huo <huobean@gmail.com> wrote on Tue, 19 May 2020 11:04:15 +0200:

> hi,  Miquel
> 
> On Mon, 2020-05-18 at 17:22 +0200, Miquel Raynal wrote:
> > Hi Bean,
> > 
> > Bean Huo <huobean@gmail.com> wrote on Mon, 18 May 2020 15:59:38
> > +0200:
> >   
> > > From: Bean Huo <beanhuo@micron.com>
> > > 
> > > After submission of patch V1 [1] and V2 [2], we stopped its update
> > > since we get
> > > stuck in the solution on how to avoid the power-loss issue in case
> > > power-cut
> > > hits the block filling. In the v1 and v2, to avoid this issue, we
> > > always damaged
> > > page0, page1, this's based on the hypothesis that NAND FS is UBIFS.
> > > This
> > > FS-specifical code is unacceptable in the MTD layer. Also, it
> > > cannot cover all
> > > NAND based file system. Based on the current discussion, seems that
> > > re-write all
> > > first 15 page from page0 is a satisfactory solution.  
> > 
> > We have a layering problem now. Maybe we should just have an MTD
> > internal variable like min_written_pages_before_erase that the Micron
> > driver could set to a !0 value.
> > 
> > Then, the handling could be done by the user (UBI/UBIFS, JFFS2, MTD
> > user if exported).
> >   
> 
> This is NAND its own problem, if no significant adantage, I don't think
> it's a good solution to extend the problem to the upper FS layer.
> also, in the FS erase path, doesn't have the programmed pages counter.
> we should repeat the same approach as we did in MTD layer.

The problem is that if the filesystem is not aware, it breaks the
"power cut safe" assertion.

There is a problem with JFFS2 and a problem with UBIFS because of that.
We can certainly keep a default implementation like this one for other
users though.

> 
> > > 
> > > Meanwhile, I borrowed one idea from Miquel Raynal patchset [3], in
> > > which keeps
> > > a recode of programmed pages, base on it, for most of the cases, we
> > > don't need
> > > to read every page to see if current erasing block is a partially
> > > programmed
> > > block.
> > > 
> > > Changelog:
> > > 
> > > v3 - v4:
> > >     1. In the patch 4/5, change to directly use ecc.strength to
> > > judge the page
> > >        is a empty page or not, rather than max_bitflips < mtd-  
> > > >bitflip_threshold  
> > >     2. In the patch 5/5, for the powerloss case, from the next time
> > > boot up,
> > >        lots of page will be programmed from >page15 address, if
> > > still using
> > >        first_p as GENMASK() bitmask starting position, writtenp
> > > will be always 0,
> > >        fix it by changing its bitmask starting at bit position 0.
> > > 
> > > v2 - v3:
> > >     1. Rebase patch to the latest MTD git tree
> > >     2. Add a record that keeps tracking the programmed pages in the
> > > first 16
> > >        pages
> > >     3. Change from program odd pages, damage page 0 and page 1, to
> > > program all
> > >        first 15 pages
> > >     4. Address issues which exist in the V2.
> > > 
> > > v1 - v2:
> > >     1. Rebased V1 to latest Linux kernel.
> > >     2. Add erase preparation function pointer in
> > > nand_manufacturer_ops.
> > > 
> > > 
> > > [1] https://www.spinics.net/lists/linux-mtd/msg04112.html
> > > [2] https://www.spinics.net/lists/linux-mtd/msg04450.html
> > > [3] https://www.spinics.net/lists/linux-mtd/msg13083.html
> > > 
> > > 
> > > Bean Huo (5):
> > >   mtd: rawnand: group all NAND specific ops into new nand_chip_ops
> > >   mtd: rawnand: Add {pre,post}_erase hooks in nand_chip_ops
> > >   mtd: rawnand: Add write_oob hook in nand_chip_ops
> > >   mtd: rawnand: Introduce a new function
> > > nand_check_is_erased_page()
> > >   mtd: rawnand: micron: Micron SLC NAND filling block  
> > 
> > When you take my patches in your series, especially when not touching
> > them at all, you should keep my Authorship and SoB first, then add
> > your
> > SoB.
> >   
> 
> sorry for my fault, I thought adding your Signed-off-by in 3/5 is
> suffient. you mean I should add your signed-off-by in 5/5 as well?
> I will do that in next version.

You should keep my Authorship and SoB for both patches + add your SoB
after mine.


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 5/5] mtd: rawnand: micron: Micron SLC NAND filling block

[Bean Huo]

On Mon, 2020-05-18 at 11:32 -0700, Steve deRosier wrote:
> On Mon, May 18, 2020 at 7:00 AM Bean Huo <huobean@gmail.com> wrote:
> > 
> > From: Bean Huo <beanhuo@micron.com>
> > 
> > On some legacy planar 2D Micron NAND devices when a block erase
> > command
> 
> I object the use of the qualifications you're putting in this
> sentence. By saying "some legacy...." you're implying that there's a
> set that does and a set that doesn't require this. Which then leads
> the reader of this commit message to #1 look for which ones this
> applies to vs not, and #2 want to remove/exclude the feature when
> they're using a "current" device. The wiggle-word wording is
> confusing
> and dishonest.
> 
> I've followed this discussion now intently and it seems like Micron
> is
> either unable or unwilling to determine which specific devices this
> does or doesn't apply to. If you are unable to identify and restrict
> this functionality to a specific subset of devices, then the fact is
> it's "all."  Let's just say that and eliminate the confusion. And
> please also update your datasheets to indicate that this is the
> correct algorithm for working with these devices. Better would be to
> issue an errata on the chips and notify your customers. I feel for
> those customers who aren't using Linux and don't know the reliability
> problem they've been tracking down for the last couple of years is
> already known but they don't have any way of knowing about it.
> 
> In your commit message, rewording to "On planar 2D Micron NAND
> devices
> when a block erase command..." is sufficient.
> 
> - Steve
> 
ok, you are native English speaker, I will take this suggestion in the
next version.

thanks.
Bean

> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/