UBI use subpage and verify fail when write on nandflash

UBI use subpage and verify fail when write on nandflash

[b w]

Hello,
    Use linux 2.6.32,ARM,nandflash(support subpage),config
CONFIG_MTD_NAND_VERIFY_WRITE. UBI attach fail on empty flash.I found
when UBI write VID header to the second subpage of eraseblock,will
call function "nand_write_page",this function write a whole page(i
think the page size is 2048),the unset bit of the buf is filled with
0xff,after write,

#ifdef CONFIG_MTD_NAND_VERIFY_WRITE
	/* Send command to read back the data */
	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);

	if (chip->verify_buf(mtd, buf, mtd->writesize))
		return -EIO;
#endif

now,the buf for verify is not equal to the buf which write,because the
buf which read from flash contains EC header ?

I found there is topic also mention subpage:
http://lists.infradead.org/pipermail/linux-mtd/2010-September/031822.html

error log:
UBI: attaching mtd3 to ubi0
UBI: physical eraseblock size:   131072 bytes (128 KiB)
UBI: logical eraseblock size:    129024 bytes
UBI: smallest flash I/O unit:    2048
UBI: sub-page size:              512
UBI: VID header offset:          512 (aligned 512)
UBI: data offset:                2048
UBI: empty MTD device detected
UBI: create volume table (copy #1)
UBI error: ubi_io_write: error -5 while writing 512 bytes to PEB
0:512, written 0 bytes
Backtrace:
[<c0212a24>] (dump_stack+0x18/0x1c)
[<c040aab8>] (ubi_io_write+0x324/0x3a8)
[<c038e118>] (ubi_io_write_vid_hdr+0x134/0x150)
[<c038e4c0>] (create_vtbl+0x1bc/0x290)
[<c0383734>] (ubi_read_volume_table+0xe8/0x984)
[<c0384070>] (ubi_attach_mtd_dev+0x690/0xd24)
[<c03873c0>] (ctrl_cdev_ioctl+0xe8/0x1a4)
[<c03883a0>] (vfs_ioctl+0x38/0x78)

Re: UBI use subpage and verify fail when write on nandflash

[Brian Norris]

Add Jon,

On Tue, Apr 17, 2012 at 7:19 AM, b w <wdjjwb@gmail.com> wrote:
> Hello,
>    Use linux 2.6.32,ARM,nandflash(support subpage),config
> CONFIG_MTD_NAND_VERIFY_WRITE. UBI attach fail on empty flash.I found
> when UBI write VID header to the second subpage of eraseblock,will
> call function "nand_write_page",this function write a whole page(i
> think the page size is 2048),the unset bit of the buf is filled with
> 0xff,after write,
>
> #ifdef CONFIG_MTD_NAND_VERIFY_WRITE
>        /* Send command to read back the data */
>        chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
>
>        if (chip->verify_buf(mtd, buf, mtd->writesize))
>                return -EIO;
> #endif
>
> now,the buf for verify is not equal to the buf which write,because the
> buf which read from flash contains EC header ?
>
> I found there is topic also mention subpage:
> http://lists.infradead.org/pipermail/linux-mtd/2010-September/031822.html

Hmm, not sure if this is exactly the same issue, but it is related.
It's worth CC'ing Jon to see if he pursued this at all.

Brian

RE: UBI use subpage and verify fail when write on nandflash

[Jon Povey]

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

Brian Norris wrote:
> On Tue, Apr 17, 2012 at 7:19 AM, b w <wdjjwb@gmail.com> wrote:
>> Hello,
>>    Use linux 2.6.32,ARM,nandflash(support subpage),config
>> CONFIG_MTD_NAND_VERIFY_WRITE. UBI attach fail on empty flash.I found
>> when UBI write VID header to the second subpage of eraseblock,will
>> call function "nand_write_page",this function write a whole page(i
>> think the page size is 2048),the unset bit of the buf is filled with
>> 0xff,after write,
>>
>> #ifdef CONFIG_MTD_NAND_VERIFY_WRITE
>>        /* Send command to read back the data */
>>        chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
>>
>>        if (chip->verify_buf(mtd, buf, mtd->writesize))
>>                return -EIO;
>> #endif
>>
>> now,the buf for verify is not equal to the buf which write,because
>> the buf which read from flash contains EC header ?
>>
>> I found there is topic also mention subpage:
>>
> http://lists.infradead.org/pipermail/linux-mtd/2010-September/
> 031822.html
>
> Hmm, not sure if this is exactly the same issue, but it is related.
> It's worth CC'ing Jon to see if he pursued this at all.

I have just carried on happily using my hack. Don't have the time to
work on a proper fix, sorry.

Hack attached if it is of interest, apologies for the format,
blame my email client. This is against roughly mainline 3.0

--
Jon Povey
jon.povey@racelogic.co.uk

Racelogic is a limited company registered in England. Registered number 2743719 .
Registered Office Unit 10, Swan Business Centre, Osier Way, Buckingham, Bucks, MK18 1TB .

The information contained in this electronic mail transmission is intended by Racelogic Ltd for the use of the named individual or entity to which it is directed and may contain information that is confidential or privileged. If you have received this electronic mail transmission in error, please delete it from your system without copying or forwarding it, and notify the sender of the error by reply email so that the sender's address records can be corrected. The views expressed by the sender of this communication do not necessarily represent those of Racelogic Ltd. Please note that Racelogic reserves the right to monitor e-mail communications passing through its network



[-- Attachment #2: 0001-mtd-NAND-don-t-write-ECC-for-unwritten-subpages.patch --]
[-- Type: application/octet-stream, Size: 7605 bytes --]

From c709d80c030ca16556128da5330034c554c239d5 Mon Sep 17 00:00:00 2001
From: Jon Povey <jon.povey@racelogic.co.uk>
Date: Tue, 25 Oct 2011 18:08:54 +0900
Subject: [PATCH] mtd: NAND: don't write ECC for unwritten subpages

Subpage writes were done by writing a whole page with unwanted areas set to FF.
This caused (non-FF) ECC to be generated and written for all subpages, meaning
two subpage writes to the same page results in corrupt ECC.
Effects included UBI errors and failure to reattach to mtd devices unless
subpage writes were disabled.

Now don't write ECC for unwanted subpages, by writing the ECC for them to FFs.

Tested on TI DaVinci DM355,DM365 with Micron 2KB page NAND flash.

Signed-off-by: Jon Povey <jon.povey@racelogic.co.uk>

Conflicts:

	drivers/mtd/nand/nand_base.c
	include/linux/mtd/nand.h
---
 drivers/mtd/nand/nand_base.c |   42 +++++++++++++++++++++++++++++++-----------
 include/linux/mtd/nand.h     |    7 ++++---
 2 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index a46e9bb..9844721 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1892,11 +1892,13 @@ out:
  * @mtd:	mtd info structure
  * @chip:	nand chip info structure
  * @buf:	data buffer
+ * @column:	start offset of valid data (ignored)
+ * @bytes:	length of valid data (ignored)
  *
  * Not for syndrome calculating ecc controllers, which use a special oob layout
  */
 static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-				const uint8_t *buf)
+				const uint8_t *buf, int column, int bytes)
 {
 	chip->write_buf(mtd, buf, mtd->writesize);
 	chip->write_buf(mtd, chip->oob_poi, mtd->oobsize);
@@ -1907,12 +1909,15 @@ static void nand_write_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd:	mtd info structure
  * @chip:	nand chip info structure
  * @buf:	data buffer
+ * @column:	start offset of valid data (ignored)
+ * @bytes:	length of valid data (ignored)
  *
  * We need a special oob layout and handling even when ECC isn't checked.
  */
 static void nand_write_page_raw_syndrome(struct mtd_info *mtd,
 					struct nand_chip *chip,
-					const uint8_t *buf)
+					const uint8_t *buf,
+					int column, int bytes)
 {
 	int eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1946,9 +1951,11 @@ static void nand_write_page_raw_syndrome(struct mtd_info *mtd,
  * @mtd:	mtd info structure
  * @chip:	nand chip info structure
  * @buf:	data buffer
+ * @column:	start offset of valid data (ignored)
+ * @bytes:	length of valid data (ignored)
  */
 static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
-				  const uint8_t *buf)
+				  const uint8_t *buf, int column, int bytes)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1964,7 +1971,7 @@ static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
 	for (i = 0; i < chip->ecc.total; i++)
 		chip->oob_poi[eccpos[i]] = ecc_calc[i];
 
-	chip->ecc.write_page_raw(mtd, chip, buf);
+	chip->ecc.write_page_raw(mtd, chip, buf, column, bytes);
 }
 
 /**
@@ -1972,9 +1979,11 @@ static void nand_write_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd:	mtd info structure
  * @chip:	nand chip info structure
  * @buf:	data buffer
+ * @column:	start offset of valid data
+ * @bytes:	length of valid data
  */
 static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
-				  const uint8_t *buf)
+				  const uint8_t *buf, int column, int bytes)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1983,10 +1992,15 @@ static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 	const uint8_t *p = buf;
 	uint32_t *eccpos = chip->ecc.layout->eccpos;
 
+	/* Hacked up by Jon, adding start and len for subpage writes */
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 		chip->ecc.hwctl(mtd, NAND_ECC_WRITE);
 		chip->write_buf(mtd, p, eccsize);
 		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
+
+		if (p - buf >= column + bytes || p - buf + eccsize <= column)
+			/* clear ECC for "unwritten" subpage */
+			memset(&ecc_calc[i], 0xff, eccbytes);
 	}
 
 	for (i = 0; i < chip->ecc.total; i++)
@@ -2000,12 +2014,15 @@ static void nand_write_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
  * @mtd:	mtd info structure
  * @chip:	nand chip info structure
  * @buf:	data buffer
+ * @column:	start offset of valid data (ignored)
+ * @bytes:	length of valid data (ignored)
  *
  * The hw generator calculates the error syndrome automatically. Therefor
  * we need a special oob layout and handling.
  */
 static void nand_write_page_syndrome(struct mtd_info *mtd,
-				    struct nand_chip *chip, const uint8_t *buf)
+				    struct nand_chip *chip, const uint8_t *buf,
+				    int column, int bytes)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -2044,21 +2061,24 @@ static void nand_write_page_syndrome(struct mtd_info *mtd,
  * @mtd:	MTD device structure
  * @chip:	NAND chip descriptor
  * @buf:	the data to write
+ * @column:	start offset of valid data
+ * @bytes:	length of valid data
  * @page:	page number to write
  * @cached:	cached programming
  * @raw:	use _raw version of write_page
  */
 static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
-			   const uint8_t *buf, int page, int cached, int raw)
+			   const uint8_t *buf, int column, int bytes, int page,
+			   int cached, int raw)
 {
 	int status;
 
 	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
-		chip->ecc.write_page_raw(mtd, chip, buf);
+		chip->ecc.write_page_raw(mtd, chip, buf, column, bytes);
 	else
-		chip->ecc.write_page(mtd, chip, buf);
+		chip->ecc.write_page(mtd, chip, buf, column, bytes);
 
 	/*
 	 * Cached progamming disabled for now, Not sure if its worth the
@@ -2230,8 +2250,8 @@ static int nand_do_write_ops(struct mtd_info *mtd, loff_t to,
 			oobwritelen -= len;
 		}
 
-		ret = chip->write_page(mtd, chip, wbuf, page, cached,
-				       (ops->mode == MTD_OOB_RAW));
+		ret = chip->write_page(mtd, chip, wbuf, column, bytes, page,
+				       cached, (ops->mode == MTD_OOB_RAW));
 		if (ret)
 			break;
 
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index c2b9ac4..6598cca 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -373,13 +373,13 @@ struct nand_ecc_ctrl {
 	int (*read_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
 			uint8_t *buf, int page);
 	void (*write_page_raw)(struct mtd_info *mtd, struct nand_chip *chip,
-			const uint8_t *buf);
+			const uint8_t *buf, int column, int bytes);
 	int (*read_page)(struct mtd_info *mtd, struct nand_chip *chip,
 			uint8_t *buf, int page);
 	int (*read_subpage)(struct mtd_info *mtd, struct nand_chip *chip,
 			uint32_t offs, uint32_t len, uint8_t *buf);
 	void (*write_page)(struct mtd_info *mtd, struct nand_chip *chip,
-			const uint8_t *buf);
+			const uint8_t *buf, int column, int bytes);
 	int (*read_oob)(struct mtd_info *mtd, struct nand_chip *chip, int page,
 			int sndcmd);
 	int (*write_oob)(struct mtd_info *mtd, struct nand_chip *chip,
@@ -505,7 +505,8 @@ struct nand_chip {
 	int (*errstat)(struct mtd_info *mtd, struct nand_chip *this, int state,
 			int status, int page);
 	int (*write_page)(struct mtd_info *mtd, struct nand_chip *chip,
-			const uint8_t *buf, int page, int cached, int raw);
+			const uint8_t *buf, int column, int bytes,
+			int page, int cached, int raw);
 
 	int chip_delay;
 	unsigned int options;
-- 
1.6.3.3

Re: UBI use subpage and verify fail when write on nandflash

[Artem Bityutskiy]

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

On Tue, 2012-04-17 at 22:19 +0800, b w wrote:
> Hello,
>     Use linux 2.6.32,ARM,nandflash(support subpage),config
> CONFIG_MTD_NAND_VERIFY_WRITE. UBI attach fail on empty flash.I found
> when UBI write VID header to the second subpage of eraseblock,will
> call function "nand_write_page",this function write a whole page(i
> think the page size is 2048),the unset bit of the buf is filled with
> 0xff,after write,

Yeah, known issue, it just waits for a fix from someone who cares
enough.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

RE: UBI use subpage and verify fail when write on nandflash

[Artem Bityutskiy]

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

On Wed, 2012-04-18 at 06:05 +0100, Jon Povey wrote:
> I have just carried on happily using my hack. Don't have the time to
> work on a proper fix, sorry.
> 
> Hack attached if it is of interest, apologies for the format,
> blame my email client. This is against roughly mainline 3.0

I really do not know why Thomas implemented it this way - probably there
was a reason. Would be nice to test your change on few more NANDs and if
it is fine - merge it.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]