linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
@ 2013-11-24 13:13 Qi Wang 王起 (qiwang)
  2013-12-03  3:05 ` Qi Wang 王起 (qiwang)
  0 siblings, 1 reply; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2013-11-24 13:13 UTC (permalink / raw)
  To: dedekind1@gmail.com, linux-mtd@lists.infradead.org
  Cc: Qi Wang 王起 (qiwang), Adrian Hunter,
	linux-kernel@vger.kernel.org

Hi:
As we talked in mail before, please check my patch as below:

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other 
operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
program operation. 

This patch try to put read operation at head of write operation in 
nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, 
just go to erase this block.

Signed-off-by: Qi Wang <qiwang@qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, need to corrupt it before erase operation.  
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
-		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if(err1)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+	    	addr += ubi->vid_hdr_aloffset;
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err1)
+			goto error;	
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.
 	 * Supposedly the flash media or the driver is screwed up, so return an
 	 * error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
 		pnum, err, err1);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
---

I have tested this patch on Micron NOR flash, part number is:JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2013-11-24 13:13 [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted Qi Wang 王起 (qiwang)
@ 2013-12-03  3:05 ` Qi Wang 王起 (qiwang)
  2013-12-05  3:48   ` Qi Wang 王起 (qiwang)
  2013-12-05  3:48   ` Qi Wang 王起 (qiwang)
  0 siblings, 2 replies; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2013-12-03  3:05 UTC (permalink / raw)
  To: dedekind1@gmail.com, linux-mtd@lists.infradead.org
  Cc: Qi Wang 王起 (qiwang), Adrian Hunter,
	linux-kernel@vger.kernel.org

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other 
operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
program operation. 

This patch try to put read operation at head of write operation in 
nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, 
just go to erase this block.

Signed-off-by: Qi Wang <qiwang@qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, need to corrupt it before erase operation.  
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
-		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if(err1)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+	    	addr += ubi->vid_hdr_aloffset;
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err1)
+			goto error;	
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.
 	 * Supposedly the flash media or the driver is screwed up, so return an
 	 * error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
 		pnum, err, err1);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
---

I have tested this patch on Micron NOR flash, part number is:JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2013-12-03  3:05 ` Qi Wang 王起 (qiwang)
@ 2013-12-05  3:48   ` Qi Wang 王起 (qiwang)
  2013-12-05  3:49     ` Qi Wang 王起 (qiwang)
  2013-12-05  3:48   ` Qi Wang 王起 (qiwang)
  1 sibling, 1 reply; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2013-12-05  3:48 UTC (permalink / raw)
  To: dedekind1@gmail.com, linux-mtd@lists.infradead.org
  Cc: Qi Wang 王起 (qiwang), Adrian Hunter,
	linux-kernel@vger.kernel.org


Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other 
operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
program operation. 

This patch try to put read operation at head of write operation in 
nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, 
just go to erase this block.

Signed-off-by: Qi Wang <qiwang@qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, need to corrupt it before erase operation.  
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
-		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if(err1)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+	    	addr += ubi->vid_hdr_aloffset;
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err1)
+			goto error;	
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.
 	 * Supposedly the flash media or the driver is screwed up, so return an
 	 * error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
 		pnum, err, err1);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
---

I have tested this patch on Micron NOR flash, part number is:JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2013-12-03  3:05 ` Qi Wang 王起 (qiwang)
  2013-12-05  3:48   ` Qi Wang 王起 (qiwang)
@ 2013-12-05  3:48   ` Qi Wang 王起 (qiwang)
  2013-12-05  3:49     ` Qi Wang 王起 (qiwang)
                       ` (2 more replies)
  1 sibling, 3 replies; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2013-12-05  3:48 UTC (permalink / raw)
  To: dedekind1@gmail.com, linux-mtd@lists.infradead.org
  Cc: Qi Wang 王起 (qiwang), Adrian Hunter,
	linux-kernel@vger.kernel.org


Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other 
operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
program operation. 

This patch try to put read operation at head of write operation in 
nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, 
just go to erase this block.

Signed-off-by: Qi Wang <qiwang@qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, need to corrupt it before erase operation.  
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
-		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if(err1)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+	    	addr += ubi->vid_hdr_aloffset;
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err1)
+			goto error;	
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.
 	 * Supposedly the flash media or the driver is screwed up, so return an
 	 * error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
 		pnum, err, err1);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
---

I have tested this patch on Micron NOR flash, part number is:JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2013-12-05  3:48   ` Qi Wang 王起 (qiwang)
@ 2013-12-05  3:49     ` Qi Wang 王起 (qiwang)
  0 siblings, 0 replies; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2013-12-05  3:49 UTC (permalink / raw)
  To: dedekind1@gmail.com, linux-mtd@lists.infradead.org
  Cc: Qi Wang 王起 (qiwang), Adrian Hunter,
	linux-kernel@vger.kernel.org

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other 
operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
program operation. 

This patch try to put read operation at head of write operation in 
nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, 
just go to erase this block.

Signed-off-by: Qi Wang <qiwang@qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, need to corrupt it before erase operation.  
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
-		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if(err1)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+	    	addr += ubi->vid_hdr_aloffset;
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err1)
+			goto error;	
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.
 	 * Supposedly the flash media or the driver is screwed up, so return an
 	 * error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
 		pnum, err, err1);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
---

I have tested this patch on Micron NOR flash, part number is:JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2013-12-05  3:48   ` Qi Wang 王起 (qiwang)
@ 2013-12-05  3:49     ` Qi Wang 王起 (qiwang)
  2013-12-10  3:15     ` Qi Wang 王起 (qiwang)
  2013-12-10  3:16     ` Qi Wang 王起 (qiwang)
  2 siblings, 0 replies; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2013-12-05  3:49 UTC (permalink / raw)
  To: dedekind1@gmail.com, linux-mtd@lists.infradead.org
  Cc: Qi Wang 王起 (qiwang), Adrian Hunter,
	linux-kernel@vger.kernel.org

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other 
operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
program operation. 

This patch try to put read operation at head of write operation in 
nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, 
just go to erase this block.

Signed-off-by: Qi Wang <qiwang@qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, need to corrupt it before erase operation.  
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
-		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if(err1)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+	    	addr += ubi->vid_hdr_aloffset;
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err1)
+			goto error;	
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.
 	 * Supposedly the flash media or the driver is screwed up, so return an
 	 * error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
 		pnum, err, err1);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
---

I have tested this patch on Micron NOR flash, part number is:JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2013-12-05  3:48   ` Qi Wang 王起 (qiwang)
  2013-12-05  3:49     ` Qi Wang 王起 (qiwang)
@ 2013-12-10  3:15     ` Qi Wang 王起 (qiwang)
  2013-12-16  4:33       ` Qi Wang 王起 (qiwang)
  2013-12-10  3:16     ` Qi Wang 王起 (qiwang)
  2 siblings, 1 reply; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2013-12-10  3:15 UTC (permalink / raw)
  To: dedekind1@gmail.com, linux-mtd@lists.infradead.org
  Cc: Qi Wang 王起 (qiwang), Adrian Hunter,
	linux-kernel@vger.kernel.org

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other 
operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
program operation. 

This patch try to put read operation at head of write operation in 
nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, 
just go to erase this block.

Signed-off-by: Qi Wang <qiwang@qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, need to corrupt it before erase operation.  
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
-		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if(err1)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+	    	addr += ubi->vid_hdr_aloffset;
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err1)
+			goto error;	
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.
 	 * Supposedly the flash media or the driver is screwed up, so return an
 	 * error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
 		pnum, err, err1);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
---

I have tested this patch on Micron NOR flash, part number is:JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* RE: [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2013-12-05  3:48   ` Qi Wang 王起 (qiwang)
  2013-12-05  3:49     ` Qi Wang 王起 (qiwang)
  2013-12-10  3:15     ` Qi Wang 王起 (qiwang)
@ 2013-12-10  3:16     ` Qi Wang 王起 (qiwang)
  2 siblings, 0 replies; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2013-12-10  3:16 UTC (permalink / raw)
  To: dedekind1@gmail.com, linux-mtd@lists.infradead.org
  Cc: Adrian Hunter, linux-kernel@vger.kernel.org

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other 
operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
program operation. 

This patch try to put read operation at head of write operation in 
nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, 
just go to erase this block.

Signed-off-by: Qi Wang <qiwang@qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, need to corrupt it before erase operation.  
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
-		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if(err1)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+	    	addr += ubi->vid_hdr_aloffset;
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err1)
+			goto error;	
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.
 	 * Supposedly the flash media or the driver is screwed up, so return an
 	 * error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
 		pnum, err, err1);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
---

I have tested this patch on Micron NOR flash, part number is:JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2013-12-10  3:15     ` Qi Wang 王起 (qiwang)
@ 2013-12-16  4:33       ` Qi Wang 王起 (qiwang)
  2013-12-23  2:03         ` Qi Wang 王起 (qiwang)
  2014-01-01 13:06         ` Qi Wang 王起 (qiwang)
  0 siblings, 2 replies; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2013-12-16  4:33 UTC (permalink / raw)
  To: dedekind1@gmail.com, linux-mtd@lists.infradead.org
  Cc: Qi Wang 王起 (qiwang), Adrian Hunter,
	linux-kernel@vger.kernel.org


Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other 
operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
program operation. 

This patch try to put read operation at head of write operation in 
nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, 
just go to erase this block.

Signed-off-by: Qi Wang <qiwang@qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, need to corrupt it before erase operation.  
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
-		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if(err1)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+	    	addr += ubi->vid_hdr_aloffset;
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err1)
+			goto error;	
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.
 	 * Supposedly the flash media or the driver is screwed up, so return an
 	 * error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
 		pnum, err, err1);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
---

I have tested this patch on Micron NOR flash, part number is:JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2013-12-16  4:33       ` Qi Wang 王起 (qiwang)
@ 2013-12-23  2:03         ` Qi Wang 王起 (qiwang)
  2014-01-01 13:06         ` Qi Wang 王起 (qiwang)
  1 sibling, 0 replies; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2013-12-23  2:03 UTC (permalink / raw)
  To: dedekind1@gmail.com
  Cc: Qi Wang 王起 (qiwang), linux-mtd@lists.infradead.org,
	Adrian Hunter, linux-kernel@vger.kernel.org


Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other 
operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
program operation. 

This patch try to put read operation at head of write operation in 
nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, 
just go to erase this block.

Signed-off-by: Qi Wang <qiwang@qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, need to corrupt it before erase operation.  
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
-		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if(err1)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+	    	addr += ubi->vid_hdr_aloffset;
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err1)
+			goto error;	
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.
 	 * Supposedly the flash media or the driver is screwed up, so return an
 	 * error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
 		pnum, err, err1);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
---

I have tested this patch on Micron NOR flash, part number is:JS28F512M29EWHA.
If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2013-12-16  4:33       ` Qi Wang 王起 (qiwang)
  2013-12-23  2:03         ` Qi Wang 王起 (qiwang)
@ 2014-01-01 13:06         ` Qi Wang 王起 (qiwang)
  2014-01-01 13:11           ` Qi Wang 王起 (qiwang)
  2014-01-02 15:11           ` Artem Bityutskiy
  1 sibling, 2 replies; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2014-01-01 13:06 UTC (permalink / raw)
  To: Qi Wang 王起 (qiwang), dedekind1@gmail.com
  Cc: linux-mtd@lists.infradead.org, Adrian Hunter,
	linux-kernel@vger.kernel.org

Hi Artem:
As we talked in mail before, please check my patch as below:

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other 
operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
program operation. 

This patch try to put read operation at head of write operation in 
nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, 
just go to erase this block.

This patch is validated on Micron NOR flash, part number is:JS28F512M29EWHA

Signed-off-by: Qi Wang <qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, need to corrupt it before erase operation.  
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
-		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if(err1)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+	    	addr += ubi->vid_hdr_aloffset;
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err1)
+			goto error;	
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.
 	 * Supposedly the flash media or the driver is screwed up, so return an
 	 * error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
 		pnum, err, err1);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
---

If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* RE: [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2014-01-01 13:06         ` Qi Wang 王起 (qiwang)
@ 2014-01-01 13:11           ` Qi Wang 王起 (qiwang)
  2014-01-02 14:58             ` Richard Weinberger
  2014-01-02 15:11           ` Artem Bityutskiy
  1 sibling, 1 reply; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2014-01-01 13:11 UTC (permalink / raw)
  To: Richard Weinberger, dedekind1@gmail.com
  Cc: Qi Wang 王起 (qiwang), linux-mtd@lists.infradead.org,
	linux-kernel@vger.kernel.org, Adrian Hunter

Hi Richard:

I modified the place you mentioned in previous mail, please check it as below:

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other 
operation on NOR flash. So UBIFS can read this block first to avoid unneeded 
program operation. 

This patch try to put read operation at head of write operation in 
nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, 
just go to erase this block.

This patch is validated on Micron NOR flash, part number is:JS28F512M29EWHA

Signed-off-by: Qi Wang <qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..0a6343d 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -499,6 +499,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,49 +510,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, need to corrupt it before erase operation.  
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
-		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if(err1)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+	    	addr += ubi->vid_hdr_aloffset;
+		err1 = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err1)
+			goto error;	
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.
 	 * Supposedly the flash media or the driver is screwed up, so return an
 	 * error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
+	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",
 		pnum, err, err1);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
---

If have any questions, please let me know. 
Thank you

Best Regards, 
Qi Wang 王起
ESG APAC AE 
Tel: 86-021-38997158
Mobile: 86-15201958202
Email: qiwang@micron.com
Address: No 601 Fasai Rd, Pudong, Shanghai, China, 200131

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted
  2014-01-01 13:11           ` Qi Wang 王起 (qiwang)
@ 2014-01-02 14:58             ` Richard Weinberger
  2014-01-03  2:16               ` Qi Wang 王起 (qiwang)
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Weinberger @ 2014-01-02 14:58 UTC (permalink / raw)
  To: Qi Wang 王起 (qiwang)
  Cc: Richard Weinberger, Adrian Hunter, linux-mtd@lists.infradead.org,
	linux-kernel@vger.kernel.org, dedekind1@gmail.com

Am Mittwoch, 1. Januar 2014, 13:11:21 schrieb Qi Wang 王起:
> If have any questions, please let me know. 

Did you send the patch again using Outlook?
Or are you behind a MS Exchange server?

The patch is still malformed. :-(
e.g. it is base64 encoded.

Can you please try using git send-email?

Thanks,
//richard

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2014-01-01 13:06         ` Qi Wang 王起 (qiwang)
  2014-01-01 13:11           ` Qi Wang 王起 (qiwang)
@ 2014-01-02 15:11           ` Artem Bityutskiy
  2014-01-02 15:22             ` Artem Bityutskiy
  2014-01-06  5:17             ` Qi Wang 王起 (qiwang)
  1 sibling, 2 replies; 23+ messages in thread
From: Artem Bityutskiy @ 2014-01-02 15:11 UTC (permalink / raw)
  To: Qi Wang 王起 (qiwang)
  Cc: linux-mtd@lists.infradead.org, Adrian Hunter,
	linux-kernel@vger.kernel.org

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

On Wed, 2014-01-01 at 13:06 +0000, Qi Wang 王起 (qiwang) wrote:
> Hi Artem:
> As we talked in mail before, please check my patch as below:

Note, because you wrote the above

> From: Qi Wang <qiwang@micron.com>

"git am" will put all the above to the commit message. If you want to
change your "From:", it should be in the first line.

Few other things.

1. If I save your e-mail as "qi.mbox" and then use "git am", I get this:

$ git am ~/tmp/qi.mbox
Applying: MTD: UBI: avoid program operation on NOR flash after erasure interrupted
/home/dedekind/git/linux-ubifs/.git/rebase-apply/patch:18: trailing whitespace.
         * If VID or EC is valid, need to corrupt it before erase operation.  
/home/dedekind/git/linux-ubifs/.git/rebase-apply/patch:63: space before tab in indent.
                addr += ubi->vid_hdr_aloffset;
/home/dedekind/git/linux-ubifs/.git/rebase-apply/patch:66: trailing whitespace.
                        goto error;
warning: 3 lines add whitespace errors.

Note, I have this in my ~/.gitconfig to notice things like that:

[core]
        whitespace = trailing-space,space-before-tab


> diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c

...

> -	 * The PEB contains a valid VID header, but we cannot invalidate it.
> +	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.

82 characters :-)

> -	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
> +	ubi_err("cannot invalidate PEB %d, read returned %d write returned %d",

Why? The error actually happened in mtd_write. No read errors lead to
this path.

>  		pnum, err, err1);

I guess it is better to kill the err1 variable and use just 'err'
everywhere.

Anyway, I did not respond for long time, so I decided to do all these
modifications myself. Could you please review and test the below patch,
which is also attached. I did not compile it even. If there are issues,
just fix-it up and send the the fix or the fixed version. If it is OK,
give me your "Tested-by:" please. Thanks!


From: =?UTF-8?q?Qi=20Wang=20=E7=8E=8B=E8=B5=B7=20=28qiwang=29?=
 <qiwang@micron.com>
Date: Wed, 1 Jan 2014 13:06:11 +0000
Subject: [PATCH] MTD: UBI: avoid program operation on NOR flash after erasure
 interrupted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other
operation on NOR flash. So UBIFS can read this block first to avoid unneeded
program operation.

This patch try to put read operation at head of write operation in
nor_erase_prepare(), read out the data.
If the data is already corrupt, then no need to program any data into this block,
just go to erase this block.

This patch is validated on Micron NOR flash, part number is:JS28F512M29EWHA

Signed-off-by: Qi Wang <qiwang@micron.com>
---
 drivers/mtd/ubi/io.c | 55 ++++++++++++++++++++++------------------------------
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..ae9e5ca 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -495,10 +495,12 @@ out:
  */
 static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 {
-	int err, err1;
+	int err;
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
+
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,50 +511,39 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, we have to corrupt them before erasing.
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
 		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+		if(err)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		addr += ubi->vid_hdr_aloffset;
+		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err)
+			goto error;
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
-	 * Supposedly the flash media or the driver is screwed up, so return an
-	 * error.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate
+	 * it. Supposedly the flash media or the driver is screwed up, so
+	 * return an error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
-		pnum, err, err1);
+	ubi_err("cannot invalidate PEB %d, write returned %d write returned %d",
+		pnum, err, err);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
 }
-- 
1.8.5.2

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: 0001-MTD-UBI-avoid-program-operation-on-NOR-flash-after-e.patch --]
[-- Type: text/x-patch, Size: 4291 bytes --]

>From 56b63ee94a82f0d813a0997fa7545b4e9ce94456 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Qi=20Wang=20=E7=8E=8B=E8=B5=B7=20=28qiwang=29?=
 <qiwang@micron.com>
Date: Wed, 1 Jan 2014 13:06:11 +0000
Subject: [PATCH] MTD: UBI: avoid program operation on NOR flash after erasure
 interrupted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other
operation on NOR flash. So UBIFS can read this block first to avoid unneeded
program operation.

This patch try to put read operation at head of write operation in
nor_erase_prepare(), read out the data.
If the data is already corrupt, then no need to program any data into this block,
just go to erase this block.

This patch is validated on Micron NOR flash, part number is:JS28F512M29EWHA

Signed-off-by: Qi Wang <qiwang@micron.com>
---
 drivers/mtd/ubi/io.c | 55 ++++++++++++++++++++++------------------------------
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..ae9e5ca 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -495,10 +495,12 @@ out:
  */
 static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 {
-	int err, err1;
+	int err;
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
+
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,50 +511,39 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, we have to corrupt them before erasing.
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
 		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+		if(err)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		addr += ubi->vid_hdr_aloffset;
+		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err)
+			goto error;
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
-	 * Supposedly the flash media or the driver is screwed up, so return an
-	 * error.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate
+	 * it. Supposedly the flash media or the driver is screwed up, so
+	 * return an error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
-		pnum, err, err1);
+	ubi_err("cannot invalidate PEB %d, write returned %d write returned %d",
+		pnum, err, err);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
 }
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2014-01-02 15:11           ` Artem Bityutskiy
@ 2014-01-02 15:22             ` Artem Bityutskiy
  2014-01-03  2:31               ` Qi Wang 王起 (qiwang)
  2014-01-06  5:17             ` Qi Wang 王起 (qiwang)
  1 sibling, 1 reply; 23+ messages in thread
From: Artem Bityutskiy @ 2014-01-02 15:22 UTC (permalink / raw)
  To: Qi Wang 王起 (qiwang)
  Cc: linux-mtd@lists.infradead.org, Adrian Hunter,
	linux-kernel@vger.kernel.org

On Thu, 2014-01-02 at 17:11 +0200, Artem Bityutskiy wrote:
> Anyway, I did not respond for long time, so I decided to do all these
> modifications myself. Could you please review and test the below patch,
> which is also attached. I did not compile it even. If there are issues,
> just fix-it up and send the the fix or the fixed version. If it is OK,
> give me your "Tested-by:" please. Thanks!

Actually, for convenience, I've pushed this patch to the 'qi' branch of
the 'linux-ubifs' git tree:

git://git.infradead.org/ubifs-2.6.git

-- 
Best Regards,
Artem Bityutskiy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2014-01-02 14:58             ` Richard Weinberger
@ 2014-01-03  2:16               ` Qi Wang 王起 (qiwang)
  0 siblings, 0 replies; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2014-01-03  2:16 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Richard Weinberger, Adrian Hunter, linux-mtd@lists.infradead.org,
	linux-kernel@vger.kernel.org, dedekind1@gmail.com

Hi Richard:
Artem already has push this patch to 'linux-ubifs' git tree.  
Thanks for you remind, I will change to git send-mail next time. 
Thanks again for  your reply. I will be very careful to push patch to you next time. 

Thanks a lot

-----Original Message-----
From: Richard Weinberger [mailto:richard@nod.at] 
Sent: Thursday, January 02, 2014 10:58 PM
To: Qi Wang 王起 (qiwang)
Cc: Richard Weinberger; dedekind1@gmail.com; linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org; Adrian Hunter
Subject: Re: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

Am Mittwoch, 1. Januar 2014, 13:11:21 schrieb Qi Wang 王起:
> If have any questions, please let me know. 

Did you send the patch again using Outlook?
Or are you behind a MS Exchange server?

The patch is still malformed. :-(
e.g. it is base64 encoded.

Can you please try using git send-email?

Thanks,
//richard


^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2014-01-02 15:22             ` Artem Bityutskiy
@ 2014-01-03  2:31               ` Qi Wang 王起 (qiwang)
  2014-01-03  5:50                 ` Artem Bityutskiy
  0 siblings, 1 reply; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2014-01-03  2:31 UTC (permalink / raw)
  To: dedekind1@gmail.com
  Cc: linux-mtd@lists.infradead.org, Adrian Hunter,
	linux-kernel@vger.kernel.org

OK, thank you Artem.

I will change to git send-mail next time. And I will be very careful to push patch to you next time.
Sorry to bring so much trouble to you as I am first time to push patch to Linux main trunk.(Maybe it also is Micron Technology company first time to push patch to Linux main trunk, I am being Micron China Shanghai team volunteer to contact Linux committee to push patch). 
Anyway, Thanks for your patience to give me a good starting. 
And if you have anything need my or my team help, Please kindly let me know. Our team is glad to contribute to Linux community.
Wish you have a wonderful 2014.
Thank you


-----Original Message-----
From: Artem Bityutskiy [mailto:dedekind1@gmail.com] 
Sent: Thursday, January 02, 2014 11:23 PM
To: Qi Wang 王起 (qiwang)
Cc: Adrian Hunter; linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org
Subject: Re: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

On Thu, 2014-01-02 at 17:11 +0200, Artem Bityutskiy wrote:
> Anyway, I did not respond for long time, so I decided to do all these
> modifications myself. Could you please review and test the below patch,
> which is also attached. I did not compile it even. If there are issues,
> just fix-it up and send the the fix or the fixed version. If it is OK,
> give me your "Tested-by:" please. Thanks!

Actually, for convenience, I've pushed this patch to the 'qi' branch of
the 'linux-ubifs' git tree:

git://git.infradead.org/ubifs-2.6.git

-- 
Best Regards,
Artem Bityutskiy


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2014-01-03  2:31               ` Qi Wang 王起 (qiwang)
@ 2014-01-03  5:50                 ` Artem Bityutskiy
  2014-01-06  5:11                   ` Qi Wang 王起 (qiwang)
                                     ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Artem Bityutskiy @ 2014-01-03  5:50 UTC (permalink / raw)
  To: Qi Wang 王起 (qiwang)
  Cc: linux-mtd@lists.infradead.org, Adrian Hunter,
	linux-kernel@vger.kernel.org

On Fri, 2014-01-03 at 02:31 +0000, Qi Wang 王起 (qiwang) wrote:
> OK, thank you Artem.
> 
> I will change to git send-mail next time. And I will be very careful to push patch to you next time.
> Sorry to bring so much trouble to you as I am first time to push patch to Linux main trunk.(Maybe it also is Micron Technology company first time to push patch to Linux main trunk, I am being Micron China Shanghai team volunteer to contact Linux committee to push patch). 
> Anyway, Thanks for your patience to give me a good starting. 
> And if you have anything need my or my team help, Please kindly let me know. Our team is glad to contribute to Linux community.
> Wish you have a wonderful 2014.

Please, remember that I pushed your patch to a temporary branch, only
for you to be able to easily pick it. I do not send it upstream.

As I said, I need to you look at it and confirm that it is all-right.
And if it is not all-right, correct it.

This is because I modified it myself.

So waiting for your response on this.

-- 
Best Regards,
Artem Bityutskiy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2014-01-03  5:50                 ` Artem Bityutskiy
@ 2014-01-06  5:11                   ` Qi Wang 王起 (qiwang)
  2014-01-06  5:16                   ` Qi Wang 王起 (qiwang)
  2014-01-09  1:35                   ` Qi Wang 王起 (qiwang)
  2 siblings, 0 replies; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2014-01-06  5:11 UTC (permalink / raw)
  To: dedekind1@gmail.com
  Cc: linux-mtd@lists.infradead.org, Adrian Hunter,
	linux-kernel@vger.kernel.org

>Please, remember that I pushed your patch to a temporary branch, only
>for you to be able to easily pick it. I do not send it upstream.

>As I said, I need to you look at it and confirm that it is all-right.
>And if it is not all-right, correct it.

>This is because I modified it myself.

>So waiting for your response on this.
I checked the code you modified, compile, and run on our platform, it is ok.

+	ubi_err("cannot invalidate PEB %d, write returned %d write returned %d",
+		pnum, err, err); 

But I think above code print "write err" twice. so I suggest to change to below:

+	ubi_err("cannot invalidate PEB %d, write returned %d", pnum, err);
 
I will send you the patch in another email convenient for you to apply patch.
Thanks
--
qiwang 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2014-01-03  5:50                 ` Artem Bityutskiy
  2014-01-06  5:11                   ` Qi Wang 王起 (qiwang)
@ 2014-01-06  5:16                   ` Qi Wang 王起 (qiwang)
  2014-01-09  1:35                   ` Qi Wang 王起 (qiwang)
  2 siblings, 0 replies; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2014-01-06  5:16 UTC (permalink / raw)
  To: dedekind1@gmail.com
  Cc: linux-mtd@lists.infradead.org, Adrian Hunter,
	linux-kernel@vger.kernel.org

From: Qi Wang <qiwang@micron.com>

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block can cause program timtout(several minutes at most) error, could impact other operation on NOR flash. So UBIFS can read this block first to avoid unneeded program operation. 

This patch try to put read operation at head of write operation in nor_erase_prepare(), read out the data. 
If the data is already corrupt, then no need to program any data into this block, just go to erase this block.

This patch is validated on Micron NOR flash, part number is:JS28F512M29EWHA

Signed-off-by: Qi Wang <qiwang@micron.com>
---
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..d361349 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -495,10 +495,12 @@ out:
  */
 static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 {
-	int err, err1;
+	int err;
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
+
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,50 +511,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, we have to corrupt them before erasing.
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
 		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+		if(err)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		addr += ubi->vid_hdr_aloffset;
+		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err)
+			goto error;
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
-	 * Supposedly the flash media or the driver is screwed up, so return an
-	 * error.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate
+	 * it. Supposedly the flash media or the driver is screwed up, so
+	 * return an error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
-		pnum, err, err1);
+	ubi_err("cannot invalidate PEB %d, write returned %d", pnum, err);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
 }
---

Compare to your patch, the change is only below:
modify from  
+	ubi_err("cannot invalidate PEB %d, write returned %d write returned %d",
+		pnum, err, err);

to 

+	ubi_err("cannot invalidate PEB %d, write returned %d", pnum, err);
Thanks


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* RE: [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2014-01-02 15:11           ` Artem Bityutskiy
  2014-01-02 15:22             ` Artem Bityutskiy
@ 2014-01-06  5:17             ` Qi Wang 王起 (qiwang)
  1 sibling, 0 replies; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2014-01-06  5:17 UTC (permalink / raw)
  To: dedekind1@gmail.com
  Cc: linux-mtd@lists.infradead.org, Adrian Hunter,
	linux-kernel@vger.kernel.org

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

Attached is my patch modified, and I use git format-patch to generate this file. Please check it. 
Thanks

-----Original Message-----
From: Artem Bityutskiy [mailto:dedekind1@gmail.com] 
Sent: Thursday, January 02, 2014 11:11 PM
To: Qi Wang 王起 (qiwang)
Cc: Adrian Hunter; linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org
Subject: Re: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

On Wed, 2014-01-01 at 13:06 +0000, Qi Wang 王起 (qiwang) wrote:
> Hi Artem:
> As we talked in mail before, please check my patch as below:

Note, because you wrote the above

> From: Qi Wang <qiwang@micron.com>

"git am" will put all the above to the commit message. If you want to change your "From:", it should be in the first line.

Few other things.

1. If I save your e-mail as "qi.mbox" and then use "git am", I get this:

$ git am ~/tmp/qi.mbox
Applying: MTD: UBI: avoid program operation on NOR flash after erasure interrupted
/home/dedekind/git/linux-ubifs/.git/rebase-apply/patch:18: trailing whitespace.
         * If VID or EC is valid, need to corrupt it before erase operation.  
/home/dedekind/git/linux-ubifs/.git/rebase-apply/patch:63: space before tab in indent.
                addr += ubi->vid_hdr_aloffset;
/home/dedekind/git/linux-ubifs/.git/rebase-apply/patch:66: trailing whitespace.
                        goto error;
warning: 3 lines add whitespace errors.

Note, I have this in my ~/.gitconfig to notice things like that:

[core]
        whitespace = trailing-space,space-before-tab


> diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c

...

> -	 * The PEB contains a valid VID header, but we cannot invalidate it.
> +	 * The PEB contains a valid VID or EC header, but we cannot invalidate it.

82 characters :-)

> -	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
> +	ubi_err("cannot invalidate PEB %d, read returned %d write returned 
> +%d",

Why? The error actually happened in mtd_write. No read errors lead to this path.

>  		pnum, err, err1);

I guess it is better to kill the err1 variable and use just 'err'
everywhere.

Anyway, I did not respond for long time, so I decided to do all these modifications myself. Could you please review and test the below patch, which is also attached. I did not compile it even. If there are issues, just fix-it up and send the the fix or the fixed version. If it is OK, give me your "Tested-by:" please. Thanks!


From: =?UTF-8?q?Qi=20Wang=20=E7=8E=8B=E8=B5=B7=20=28qiwang=29?=
 <qiwang@micron.com>
Date: Wed, 1 Jan 2014 13:06:11 +0000
Subject: [PATCH] MTD: UBI: avoid program operation on NOR flash after erasure  interrupted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block can cause program timtout(several minutes at most) error, could impact other operation on NOR flash. So UBIFS can read this block first to avoid unneeded program operation.

This patch try to put read operation at head of write operation in nor_erase_prepare(), read out the data.
If the data is already corrupt, then no need to program any data into this block, just go to erase this block.

This patch is validated on Micron NOR flash, part number is:JS28F512M29EWHA

Signed-off-by: Qi Wang <qiwang@micron.com>
---
 drivers/mtd/ubi/io.c | 55 ++++++++++++++++++++++------------------------------
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index bf79def..ae9e5ca 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -495,10 +495,12 @@ out:
  */
 static int nor_erase_prepare(struct ubi_device *ubi, int pnum)  {
-	int err, err1;
+	int err;
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
+
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header @@ -509,50 +511,39 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, we have to corrupt them before erasing.
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
 		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+		if(err)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		addr += ubi->vid_hdr_aloffset;
+		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err)
+			goto error;
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
-	 * Supposedly the flash media or the driver is screwed up, so return an
-	 * error.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate
+	 * it. Supposedly the flash media or the driver is screwed up, so
+	 * return an error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
-		pnum, err, err1);
+	ubi_err("cannot invalidate PEB %d, write returned %d write returned %d",
+		pnum, err, err);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
 }
--
1.8.5.2

--
Best Regards,
Artem Bityutskiy

[-- Attachment #2: 0001-MTD-UBI-avoid-program-operation-on-NOR-flash-after-e.patch --]
[-- Type: application/octet-stream, Size: 4262 bytes --]

From 174239fcac79f867d0f7977a1e5b7b74d6710e31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?"Qi=20Wang=20=E7=8E=8B=E8=B5=B7=20(qiwang)"?=
 <qiwang@micron.com>
Date: Wed, 1 Jan 2014 13:06:11 +0000
Subject: [PATCH] MTD: UBI: avoid program operation on NOR flash after erasure
 interrupted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

nor_erase_prepare() will be called before erase a NOR flash, it will program '0'
into a block to mark this block. But program data into a erasure interrupted block
can cause program timtout(several minutes at most) error, could impact other
operation on NOR flash. So UBIFS can read this block first to avoid unneeded
program operation.

This patch try to put read operation at head of write operation in
nor_erase_prepare(), read out the data.
If the data is already corrupt, then no need to program any data into this block,
just go to erase this block.

This patch is validated on Micron NOR flash, part number is:JS28F512M29EWHA

Signed-off-by: Qi Wang <qiwang@micron.com>
---
 drivers/mtd/ubi/io.c |   54 ++++++++++++++++++++------------------------------
 1 file changed, 22 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index bf79def..d361349 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -495,10 +495,12 @@ out:
  */
 static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 {
-	int err, err1;
+	int err;
 	size_t written;
 	loff_t addr;
 	uint32_t data = 0;
+	struct ubi_ec_hdr ec_hdr;
+
 	/*
 	 * Note, we cannot generally define VID header buffers on stack,
 	 * because of the way we deal with these buffers (see the header
@@ -509,50 +511,38 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	struct ubi_vid_hdr vid_hdr;
 
 	/*
+	 * If VID or EC is valid, we have to corrupt them before erasing.
 	 * It is important to first invalidate the EC header, and then the VID
 	 * header. Otherwise a power cut may lead to valid EC header and
 	 * invalid VID header, in which case UBI will treat this PEB as
 	 * corrupted and will try to preserve it, and print scary warnings.
 	 */
 	addr = (loff_t)pnum * ubi->peb_size;
-	err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-	if (!err) {
-		addr += ubi->vid_hdr_aloffset;
+	err = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
 		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
-		if (!err)
-			return 0;
+		if(err)
+			goto error;
 	}
 
-	/*
-	 * We failed to write to the media. This was observed with Spansion
-	 * S29GL512N NOR flash. Most probably the previously eraseblock erasure
-	 * was interrupted at a very inappropriate moment, so it became
-	 * unwritable. In this case we probably anyway have garbage in this
-	 * PEB.
-	 */
-	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-	    err1 == UBI_IO_FF) {
-		struct ubi_ec_hdr ec_hdr;
-
-		err1 = ubi_io_read_ec_hdr(ubi, pnum, &ec_hdr, 0);
-		if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR ||
-		    err1 == UBI_IO_FF)
-			/*
-			 * Both VID and EC headers are corrupted, so we can
-			 * safely erase this PEB and not afraid that it will be
-			 * treated as a valid PEB in case of an unclean reboot.
-			 */
-			return 0;
+	err = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
+	if (err != UBI_IO_BAD_HDR_EBADMSG && err != UBI_IO_BAD_HDR &&
+	    err != UBI_IO_FF){
+		addr += ubi->vid_hdr_aloffset;
+		err = mtd_write(ubi->mtd, addr, 4, &written, (void *)&data);
+		if (err)
+			goto error;
 	}
+	return 0;
 
+error:
 	/*
-	 * The PEB contains a valid VID header, but we cannot invalidate it.
-	 * Supposedly the flash media or the driver is screwed up, so return an
-	 * error.
+	 * The PEB contains a valid VID or EC header, but we cannot invalidate
+	 * it. Supposedly the flash media or the driver is screwed up, so
+	 * return an error.
 	 */
-	ubi_err("cannot invalidate PEB %d, write returned %d read returned %d",
-		pnum, err, err1);
+	ubi_err("cannot invalidate PEB %d, write returned %d", pnum, err);
 	ubi_dump_flash(ubi, pnum, 0, ubi->peb_size);
 	return -EIO;
 }
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* RE: [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2014-01-03  5:50                 ` Artem Bityutskiy
  2014-01-06  5:11                   ` Qi Wang 王起 (qiwang)
  2014-01-06  5:16                   ` Qi Wang 王起 (qiwang)
@ 2014-01-09  1:35                   ` Qi Wang 王起 (qiwang)
  2014-01-09 16:28                     ` Artem Bityutskiy
  2 siblings, 1 reply; 23+ messages in thread
From: Qi Wang 王起 (qiwang) @ 2014-01-09  1:35 UTC (permalink / raw)
  To: dedekind1@gmail.com
  Cc: Qi Wang 王起 (qiwang), linux-mtd@lists.infradead.org,
	Adrian Hunter, linux-kernel@vger.kernel.org

Hi Artem:
Do you have any suggestion for my modification?
Other code I think it is ok.
Thanks


-----Original Message-----
From: Qi Wang 王起 (qiwang) 
Sent: Monday, January 06, 2014 1:11 PM
To: 'dedekind1@gmail.com'
Cc: Adrian Hunter; linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org
Subject: RE: [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted

>Please, remember that I pushed your patch to a temporary branch, only
>for you to be able to easily pick it. I do not send it upstream.

>As I said, I need to you look at it and confirm that it is all-right.
>And if it is not all-right, correct it.

>This is because I modified it myself.

>So waiting for your response on this.
I checked the code you modified, compile, and run on our platform, it is ok.

+	ubi_err("cannot invalidate PEB %d, write returned %d write returned %d",
+		pnum, err, err); 

But I think above code print "write err" twice. so I suggest to change to below:

+	ubi_err("cannot invalidate PEB %d, write returned %d", pnum, err);
 
I will send you the patch in another email convenient for you to apply patch.
Thanks
--
qiwang 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/1]  MTD: UBI:  avoid program operation on  NOR flash after erasure interrupted
  2014-01-09  1:35                   ` Qi Wang 王起 (qiwang)
@ 2014-01-09 16:28                     ` Artem Bityutskiy
  0 siblings, 0 replies; 23+ messages in thread
From: Artem Bityutskiy @ 2014-01-09 16:28 UTC (permalink / raw)
  To: Qi Wang 王起 (qiwang)
  Cc: linux-mtd@lists.infradead.org, Adrian Hunter,
	linux-kernel@vger.kernel.org

On Thu, 2014-01-09 at 01:35 +0000, Qi Wang 王起 (qiwang) wrote:
> Hi Artem:
> Do you have any suggestion for my modification?
> Other code I think it is ok.

It is taken and pushed out. Thanks!

-- 
Best Regards,
Artem Bityutskiy

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2014-01-09 16:28 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-24 13:13 [PATCH 1/1] MTD: UBI: avoid program operation on NOR flash after erasure interrupted Qi Wang 王起 (qiwang)
2013-12-03  3:05 ` Qi Wang 王起 (qiwang)
2013-12-05  3:48   ` Qi Wang 王起 (qiwang)
2013-12-05  3:49     ` Qi Wang 王起 (qiwang)
2013-12-05  3:48   ` Qi Wang 王起 (qiwang)
2013-12-05  3:49     ` Qi Wang 王起 (qiwang)
2013-12-10  3:15     ` Qi Wang 王起 (qiwang)
2013-12-16  4:33       ` Qi Wang 王起 (qiwang)
2013-12-23  2:03         ` Qi Wang 王起 (qiwang)
2014-01-01 13:06         ` Qi Wang 王起 (qiwang)
2014-01-01 13:11           ` Qi Wang 王起 (qiwang)
2014-01-02 14:58             ` Richard Weinberger
2014-01-03  2:16               ` Qi Wang 王起 (qiwang)
2014-01-02 15:11           ` Artem Bityutskiy
2014-01-02 15:22             ` Artem Bityutskiy
2014-01-03  2:31               ` Qi Wang 王起 (qiwang)
2014-01-03  5:50                 ` Artem Bityutskiy
2014-01-06  5:11                   ` Qi Wang 王起 (qiwang)
2014-01-06  5:16                   ` Qi Wang 王起 (qiwang)
2014-01-09  1:35                   ` Qi Wang 王起 (qiwang)
2014-01-09 16:28                     ` Artem Bityutskiy
2014-01-06  5:17             ` Qi Wang 王起 (qiwang)
2013-12-10  3:16     ` Qi Wang 王起 (qiwang)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).