Add support for BBT without touching OOB area

Add support for BBT without touching OOB area

[Sebastian Andrzej Siewior]

Without this patch (number three) NAND controllers which use complete OOB
area for ECC information can not use a BBT at all. The vendor bit is
gone after the first write and the flash BBT stores a marker in the OOB
area. The patch simply moves the marker into the data area.

The series contains:
- [PATCH 1/5] mtd/nand: use ALIGN where possible
- [PATCH 2/5] mtd/nand: pull in td into read_bbt()
clean up.

- [PATCH 3/5] mtd/nand: add support for BBT without OOB
adds the functionality.

- [PATCH 4/5] mtd/nand: introduce NAND_CREATE_EMPTY_BBT
NAND_CREATE_BBT creates a BBT based on vendor's informations (or something
else if specified). Without NAND_CREATE_BBT the BBT is written to flash
after the first bad block is found. This options enforces writting an
"empty" table.

- [PATCH 5/5] mtd/nandsim: add module param for BBT handling
Used for first testing.

Sebastian

[PATCH 1/5] mtd/nand: use ALIGN where possible

[Sebastian Andrzej Siewior]

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mtd/nand/nand_bbt.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 5fedf4a..a2a4e17 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -737,8 +737,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
 			/* Calc length */
 			len = (size_t) (numblocks >> sft);
 			/* Make it page aligned ! */
-			len = (len + (mtd->writesize - 1)) &
-				~(mtd->writesize - 1);
+			len = ALIGN(len, mtd->writesize);
 			/* Preset the buffer with 0xff */
 			memset(buf, 0xff, len +
 			       (len >> this->page_shift)* mtd->oobsize);
-- 
1.7.2.3

[PATCH 2/5] mtd/nand: pull in td into read_bbt()

[Sebastian Andrzej Siewior]

No code change.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mtd/nand/nand_bbt.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index a2a4e17..587297e 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -161,21 +161,22 @@ static int check_short_pattern(uint8_t *buf, struct nand_bbt_descr *td)
  * @buf:	temporary buffer
  * @page:	the starting page
  * @num:	the number of bbt descriptors to read
- * @bits:	number of bits per block
+ * @td:		the bbt describtion table
  * @offs:	offset in the memory table
- * @reserved_block_code:	Pattern to identify reserved blocks
  *
  * Read the bad block table starting from page.
  *
  */
 static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,
-		    int bits, int offs, int reserved_block_code)
+		struct nand_bbt_descr *td, int offs)
 {
 	int res, i, j, act = 0;
 	struct nand_chip *this = mtd->priv;
 	size_t retlen, len, totlen;
 	loff_t from;
+	int bits = td->options & NAND_BBT_NRBITS_MSK;
 	uint8_t msk = (uint8_t) ((1 << bits) - 1);
+	int reserved_block_code = td->reserved_block_code;
 
 	totlen = (num * bits) >> 3;
 	from = ((loff_t) page) << this->page_shift;
@@ -238,20 +239,21 @@ static int read_abs_bbt(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_desc
 {
 	struct nand_chip *this = mtd->priv;
 	int res = 0, i;
-	int bits;
 
-	bits = td->options & NAND_BBT_NRBITS_MSK;
 	if (td->options & NAND_BBT_PERCHIP) {
 		int offs = 0;
 		for (i = 0; i < this->numchips; i++) {
 			if (chip == -1 || chip == i)
-				res = read_bbt (mtd, buf, td->pages[i], this->chipsize >> this->bbt_erase_shift, bits, offs, td->reserved_block_code);
+				res = read_bbt(mtd, buf, td->pages[i],
+					this->chipsize >> this->bbt_erase_shift,
+					td, offs);
 			if (res)
 				return res;
 			offs += this->chipsize >> (this->bbt_erase_shift + 2);
 		}
 	} else {
-		res = read_bbt (mtd, buf, td->pages[0], mtd->size >> this->bbt_erase_shift, bits, 0, td->reserved_block_code);
+		res = read_bbt(mtd, buf, td->pages[0],
+				mtd->size >> this->bbt_erase_shift, td, 0);
 		if (res)
 			return res;
 	}
-- 
1.7.2.3

[PATCH 3/5] mtd/nand: add support for BBT without OOB

[Sebastian Andrzej Siewior]

The first (sixt) byte in the OOB area contains vendor's bad block
information. During identification of the NAND chip this information is
collected by scanning the complete chip.
The option NAND_USE_FLASH_BBT is used to store this information in a sector so
we don't have to scan the complete flash. Unfortunately the code stores
a marker, in order to recognize the BBT, in the OOB area. This will fail
if the OOB area is completely used for ECC.
This patch introduces the option NAND_USE_FLASH_BBT_NO_OOB which has to be
used with NAND_USE_FLASH_BBT. It will then store BBT on flash without
touching the OOB area. The BBT format on flash remains same except the
first page starts with the recognition pattern followed by the no longer
optional version byte.
This change was tested in nandsim and on real HW with this issue.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mtd/nand/nand_bbt.c |  193 ++++++++++++++++++++++++++++++++++++++++---
 include/linux/mtd/bbm.h     |    2 +
 include/linux/mtd/nand.h    |    7 +-
 3 files changed, 189 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 587297e..11e4204 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -59,6 +59,16 @@
 #include <linux/delay.h>
 #include <linux/vmalloc.h>
 
+static int check_pattern_no_oob(uint8_t *buf, struct nand_bbt_descr *td)
+{
+	int ret;
+
+	ret = memcmp(buf, td->pattern, td->len);
+	if (!ret)
+		return ret;
+	return -1;
+}
+
 /**
  * check_pattern - [GENERIC] check if a pattern is in the buffer
  * @buf:	the buffer to search
@@ -77,6 +87,9 @@ static int check_pattern(uint8_t *buf, int len, int paglen, struct nand_bbt_desc
 	int i, end = 0;
 	uint8_t *p = buf;
 
+	if (td->options & NAND_BBT_NO_OOB)
+		return check_pattern_no_oob(buf, td);
+
 	end = paglen + td->offs;
 	if (td->options & NAND_BBT_SCANEMPTY) {
 		for (i = 0; i < end; i++) {
@@ -156,6 +169,25 @@ static int check_short_pattern(uint8_t *buf, struct nand_bbt_descr *td)
 }
 
 /**
+ * add_marker_len - compute the length of the marker in data area
+ * @td:		BBT descriptor used for computation
+ *
+ * The length will be 0 if the markeris located in OOB area.
+ */
+static u32 add_marker_len(struct nand_bbt_descr *td)
+{
+	u32 len;
+
+	if (!(td->options & NAND_BBT_NO_OOB))
+		return 0;
+
+	len = td->len;
+	if (td->options & NAND_BBT_VERSION)
+		len++;
+	return len;
+}
+
+/**
  * read_bbt - [GENERIC] Read the bad block table starting from page
  * @mtd:	MTD device structure
  * @buf:	temporary buffer
@@ -176,13 +208,26 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,
 	loff_t from;
 	int bits = td->options & NAND_BBT_NRBITS_MSK;
 	uint8_t msk = (uint8_t) ((1 << bits) - 1);
+	u32 marker_len;
 	int reserved_block_code = td->reserved_block_code;
 
 	totlen = (num * bits) >> 3;
+	marker_len = add_marker_len(td);
 	from = ((loff_t) page) << this->page_shift;
 
 	while (totlen) {
+
 		len = min(totlen, (size_t) (1 << this->bbt_erase_shift));
+
+		if (marker_len) {
+			/*
+			 * In case the BBT marker is not in the OOB area it
+			 * will be just in the first page
+			 */
+			len -= marker_len;
+			from += marker_len;
+			marker_len = 0;
+		}
 		res = mtd->read(mtd, from, len, &retlen, buf);
 		if (res < 0) {
 			if (retlen != len) {
@@ -261,9 +306,25 @@ static int read_abs_bbt(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_desc
 }
 
 /*
+ * BBT marker is in the first page, no OOB
+ */
+static int scan_read_raw_data(struct mtd_info *mtd, uint8_t *buf, loff_t offs,
+			 struct nand_bbt_descr *td)
+{
+	size_t retlen;
+	size_t len;
+
+	len = td->len;
+	if (td->options & NAND_BBT_VERSION)
+		len++;
+
+	return mtd->read(mtd, offs, len, &retlen, buf);
+}
+
+/*
  * Scan read raw data from flash
  */
-static int scan_read_raw(struct mtd_info *mtd, uint8_t *buf, loff_t offs,
+static int scan_read_raw_oob(struct mtd_info *mtd, uint8_t *buf, loff_t offs,
 			 size_t len)
 {
 	struct mtd_oob_ops ops;
@@ -296,6 +357,15 @@ static int scan_read_raw(struct mtd_info *mtd, uint8_t *buf, loff_t offs,
 	return 0;
 }
 
+static int scan_read_raw(struct mtd_info *mtd, uint8_t *buf, loff_t offs,
+			 size_t len, struct nand_bbt_descr *td)
+{
+	if (td->options & NAND_BBT_NO_OOB)
+		return scan_read_raw_data(mtd, buf, offs, td);
+	else
+		return scan_read_raw_oob(mtd, buf, offs, len);
+}
+
 /*
  * Scan write data with oob to flash
  */
@@ -314,6 +384,15 @@ static int scan_write_bbt(struct mtd_info *mtd, loff_t offs, size_t len,
 	return mtd->write_oob(mtd, offs, &ops);
 }
 
+static u32 bbt_get_ver_offs(struct mtd_info *mtd, struct nand_bbt_descr *td)
+{
+	u32 ver_offs = td->veroffs;
+
+	if (!(td->options & NAND_BBT_NO_OOB))
+		ver_offs += mtd->writesize;
+	return ver_offs;
+}
+
 /**
  * read_abs_bbts - [GENERIC] Read the bad block table(s) for all chips starting at a given page
  * @mtd:	MTD device structure
@@ -333,8 +412,8 @@ static int read_abs_bbts(struct mtd_info *mtd, uint8_t *buf,
 	/* Read the primary version, if available */
 	if (td->options & NAND_BBT_VERSION) {
 		scan_read_raw(mtd, buf, (loff_t)td->pages[0] << this->page_shift,
-			      mtd->writesize);
-		td->version[0] = buf[mtd->writesize + td->veroffs];
+			      mtd->writesize, td);
+		td->version[0] = buf[bbt_get_ver_offs(mtd, td)];
 		printk(KERN_DEBUG "Bad block table at page %d, version 0x%02X\n",
 		       td->pages[0], td->version[0]);
 	}
@@ -342,8 +421,8 @@ static int read_abs_bbts(struct mtd_info *mtd, uint8_t *buf,
 	/* Read the mirror version, if available */
 	if (md && (md->options & NAND_BBT_VERSION)) {
 		scan_read_raw(mtd, buf, (loff_t)md->pages[0] << this->page_shift,
-			      mtd->writesize);
-		md->version[0] = buf[mtd->writesize + md->veroffs];
+			      mtd->writesize, td);
+		md->version[0] = buf[bbt_get_ver_offs(mtd, md)];
 		printk(KERN_DEBUG "Bad block table at page %d, version 0x%02X\n",
 		       md->pages[0], md->version[0]);
 	}
@@ -359,7 +438,7 @@ static int scan_block_full(struct mtd_info *mtd, struct nand_bbt_descr *bd,
 {
 	int ret, j;
 
-	ret = scan_read_raw(mtd, buf, offs, readlen);
+	ret = scan_read_raw_oob(mtd, buf, offs, readlen);
 	if (ret)
 		return ret;
 
@@ -466,6 +545,8 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
 	for (i = startblock; i < numblocks;) {
 		int ret;
 
+		BUG_ON(bd->options & NAND_BBT_NO_OOB);
+
 		if (bd->options & NAND_BBT_SCANALLPAGES)
 			ret = scan_block_full(mtd, bd, from, buf, readlen,
 					      scanlen, len);
@@ -547,11 +628,12 @@ static int search_bbt(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_descr
 			loff_t offs = (loff_t)actblock << this->bbt_erase_shift;
 
 			/* Read first page */
-			scan_read_raw(mtd, buf, offs, mtd->writesize);
+			scan_read_raw(mtd, buf, offs, mtd->writesize, td);
 			if (!check_pattern(buf, scanlen, mtd->writesize, td)) {
 				td->pages[i] = actblock << blocktopage;
 				if (td->options & NAND_BBT_VERSION) {
-					td->version[i] = buf[mtd->writesize + td->veroffs];
+					u32 ver_offs = bbt_get_ver_offs(mtd, td);
+					td->version[i] = buf[ver_offs];
 				}
 				break;
 			}
@@ -735,6 +817,23 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
 			memset(&buf[offs], 0xff, (size_t) (numblocks >> sft));
 			ooboffs = len + (pageoffs * mtd->oobsize);
 
+		} else if (td->options & NAND_BBT_NO_OOB) {
+
+			ooboffs = 0;
+			offs = td->len;
+			/* the version byte */
+			if (td->options & NAND_BBT_VERSION)
+				offs++;
+			/* Calc length */
+			len = (size_t) (numblocks >> sft);
+			len += offs;
+			/* Make it page aligned ! */
+			len = ALIGN(len, mtd->writesize);
+			/* Preset the buffer with 0xff */
+			memset(buf, 0xff, len);
+			/* Pattern is located at the begin of first page */
+			memcpy(buf, td->pattern, td->len);
+
 		} else {
 			/* Calc length */
 			len = (size_t) (numblocks >> sft);
@@ -773,7 +872,9 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
 		if (res < 0)
 			goto outerr;
 
-		res = scan_write_bbt(mtd, to, len, buf, &buf[len]);
+		res = scan_write_bbt(mtd, to, len, buf,
+				td->options & NAND_BBT_NO_OOB ? NULL :
+				&buf[len]);
 		if (res < 0)
 			goto outerr;
 
@@ -984,6 +1085,49 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
 }
 
 /**
+ * verify_bbt_descr - verify the bad block description
+ * @bd:			the table to verify
+ *
+ * This functions performs a few sanity check on the bad block description
+ * table.
+ */
+static void verify_bbt_descr(struct mtd_info *mtd, struct nand_bbt_descr *bd)
+{
+	struct nand_chip *this = mtd->priv;
+	u32 pattern_len = bd->len;
+	u32 bits = bd->options & NAND_BBT_NRBITS_MSK;
+	u32 table_size;
+
+	if (!bd)
+		return;
+	BUG_ON((this->options & NAND_USE_FLASH_BBT_NO_OOB) &&
+			!(this->options & NAND_USE_FLASH_BBT));
+	BUG_ON(!bits);
+
+	if (bd->options & NAND_BBT_VERSION)
+		pattern_len++;
+
+	if (bd->options & NAND_BBT_NO_OOB) {
+		BUG_ON(!(this->options & NAND_USE_FLASH_BBT));
+		BUG_ON(!(this->options & NAND_USE_FLASH_BBT_NO_OOB));
+		BUG_ON(bd->offs);
+		if (bd->options & NAND_BBT_VERSION)
+			BUG_ON(bd->veroffs != bd->len);
+		BUG_ON(bd->options & NAND_BBT_SAVECONTENT);
+	}
+
+	if (bd->options & NAND_BBT_PERCHIP)
+		table_size = this->chipsize >> this->bbt_erase_shift;
+	else
+		table_size = mtd->size >> this->bbt_erase_shift;
+	table_size >>= 3;
+	table_size *= bits;
+	if (bd->options & NAND_BBT_NO_OOB)
+		table_size += pattern_len;
+	BUG_ON(table_size > (1 << this->bbt_erase_shift));
+}
+
+/**
  * nand_scan_bbt - [NAND Interface] scan, find, read and maybe create bad block table(s)
  * @mtd:	MTD device structure
  * @bd:		descriptor for the good/bad block search pattern
@@ -1024,6 +1168,8 @@ int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
 		}
 		return res;
 	}
+	verify_bbt_descr(mtd, td);
+	verify_bbt_descr(mtd, md);
 
 	/* Allocate a temporary buffer for one eraseblock incl. oob */
 	len = (1 << this->bbt_erase_shift);
@@ -1167,6 +1313,26 @@ static struct nand_bbt_descr bbt_mirror_descr = {
 	.pattern = mirror_pattern
 };
 
+static struct nand_bbt_descr bbt_main_no_bbt_descr = {
+	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
+		| NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP
+		| NAND_BBT_NO_OOB,
+	.len = 4,
+	.veroffs = 4,
+	.maxblocks = 4,
+	.pattern = bbt_pattern
+};
+
+static struct nand_bbt_descr bbt_mirror_no_bbt_descr = {
+	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
+		| NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP
+		| NAND_BBT_NO_OOB,
+	.len = 4,
+	.veroffs = 4,
+	.maxblocks = 4,
+	.pattern = mirror_pattern
+};
+
 #define BBT_SCAN_OPTIONS (NAND_BBT_SCANLASTPAGE | NAND_BBT_SCAN2NDPAGE | \
 		NAND_BBT_SCANBYTE1AND6)
 /**
@@ -1237,8 +1403,13 @@ int nand_default_bbt(struct mtd_info *mtd)
 	if (this->options & NAND_USE_FLASH_BBT) {
 		/* Use the default pattern descriptors */
 		if (!this->bbt_td) {
-			this->bbt_td = &bbt_main_descr;
-			this->bbt_md = &bbt_mirror_descr;
+			if (this->options & NAND_USE_FLASH_BBT_NO_OOB) {
+				this->bbt_td = &bbt_main_no_bbt_descr;
+				this->bbt_md = &bbt_mirror_no_bbt_descr;
+			} else {
+				this->bbt_td = &bbt_main_descr;
+				this->bbt_md = &bbt_mirror_descr;
+			}
 		}
 		if (!this->badblock_pattern) {
 			this->badblock_pattern = (mtd->writesize > 512) ? &largepage_flashbased : &smallpage_flashbased;
diff --git a/include/linux/mtd/bbm.h b/include/linux/mtd/bbm.h
index 7fa20be..ba15b52 100644
--- a/include/linux/mtd/bbm.h
+++ b/include/linux/mtd/bbm.h
@@ -102,6 +102,8 @@ struct nand_bbt_descr {
 #define NAND_BBT_SCANBYTE1AND6 0x00100000
 /* The nand_bbt_descr was created dynamicaly and must be freed */
 #define NAND_BBT_DYNAMICSTRUCT 0x00200000
+/* The bad block table does not OOB for marker */
+#define NAND_BBT_NO_OOB		0x00400000
 
 /* The maximum number of blocks to scan for a bbt */
 #define NAND_BBT_SCAN_MAXBLOCKS	4
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 102e12c..68caf20 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -205,8 +205,8 @@ typedef enum {
 #define NAND_CHIPOPTIONS_MSK	(0x0000ffff & ~NAND_NO_AUTOINCR)
 
 /* Non chip related options */
-/* Use a flash based bad block table. This option is passed to the
- * default bad block table function. */
+/* Use a flash based bad block table. OOB identifier is saved in OOB area.
+ * This option is passed to the default bad block table function. */
 #define NAND_USE_FLASH_BBT	0x00010000
 /* This option skips the bbt scan during initialization. */
 #define NAND_SKIP_BBTSCAN	0x00020000
@@ -215,6 +215,9 @@ typedef enum {
 #define NAND_OWN_BUFFERS	0x00040000
 /* Chip may not exist, so silence any errors in scan */
 #define NAND_SCAN_SILENT_NODEV	0x00080000
+/* If passed additionally to NAND_USE_FLASH_BBT then BBT code will not touch
+ * the OOB area */
+#define NAND_USE_FLASH_BBT_NO_OOB	0x00100000
 
 /* Options set by nand scan */
 /* Nand scan has allocated controller struct */
-- 
1.7.2.3

Re: [PATCH 3/5] mtd/nand: add support for BBT without OOB

[Artem Bityutskiy]

On Wed, 2010-09-29 at 19:43 +0200, Sebastian Andrzej Siewior wrote:
> The first (sixt) byte in the OOB area contains vendor's bad block
> information. During identification of the NAND chip this information is
> collected by scanning the complete chip.
> The option NAND_USE_FLASH_BBT is used to store this information in a sector so
> we don't have to scan the complete flash. Unfortunately the code stores
> a marker, in order to recognize the BBT, in the OOB area. This will fail
> if the OOB area is completely used for ECC.
> This patch introduces the option NAND_USE_FLASH_BBT_NO_OOB which has to be
> used with NAND_USE_FLASH_BBT. It will then store BBT on flash without
> touching the OOB area. The BBT format on flash remains same except the
> first page starts with the recognition pattern followed by the no longer
> optional version byte.
> This change was tested in nandsim and on real HW with this issue.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  drivers/mtd/nand/nand_bbt.c |  193 ++++++++++++++++++++++++++++++++++++++++---
>  include/linux/mtd/bbm.h     |    2 +
>  include/linux/mtd/nand.h    |    7 +-
>  3 files changed, 189 insertions(+), 13 deletions(-)

Few minor things.

First, is it please possible to add more descriptive comment into the
code. Something like you put to the patch description, may be even a bit
more descriptive. May be to the header of nand_bbt.c?

The idea is that the BBT code is quite difficult to follow, and you make
it even more complex, so a comment describing various flavors of BBT
would be very useful.

And then several stylistic nitpicks.

>  	while (totlen) {
> +
>  		len = min(totlen, (size_t) (1 << this->bbt_erase_shift));

These extra whits spaces are not really needed. Ideally, your patch
should make checkpatch.pl happy.
> +
> +		if (marker_len) {
> +			/*
> +			 * In case the BBT marker is not in the OOB area it
> +			 * will be just in the first page
> +			 */

Yes, this is the right multi-line comment. You may also put a dot at the
end. But in other places you used not the right multi-line comments.

> +		} else if (td->options & NAND_BBT_NO_OOB) {
> +
> +			ooboffs = 0;

Extra blank line.

> +			offs = td->len;
> +			/* the version byte */
> +			if (td->options & NAND_BBT_VERSION)
> +				offs++;
> +			/* Calc length */
> +			len = (size_t) (numblocks >> sft);
> +			len += offs;
> +			/* Make it page aligned ! */
> +			len = ALIGN(len, mtd->writesize);
> +			/* Preset the buffer with 0xff */
> +			memset(buf, 0xff, len);
> +			/* Pattern is located at the begin of first page */
> +			memcpy(buf, td->pattern, td->len);
> +
>  		} else {

And here.

> -/* Use a flash based bad block table. This option is passed to the
> - * default bad block table function. */
> +/* Use a flash based bad block table. OOB identifier is saved in OOB area.
> + * This option is passed to the default bad block table function. */

Would be nice to convert this to the right multi-line comment.

>  #define NAND_USE_FLASH_BBT	0x00010000
>  /* This option skips the bbt scan during initialization. */
>  #define NAND_SKIP_BBTSCAN	0x00020000
> @@ -215,6 +215,9 @@ typedef enum {
>  #define NAND_OWN_BUFFERS	0x00040000
>  /* Chip may not exist, so silence any errors in scan */
>  #define NAND_SCAN_SILENT_NODEV	0x00080000
> +/* If passed additionally to NAND_USE_FLASH_BBT then BBT code will not touch
> + * the OOB area */

And this. Yeah, I know this file uses different styles, but you could
convert even all of them.

> +#define NAND_USE_FLASH_BBT_NO_OOB	0x00100000

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [PATCH 3/5] mtd/nand: add support for BBT without OOB

[Sebastian Andrzej Siewior]

* Artem Bityutskiy | 2010-09-29 23:11:04 [+0300]:

>Few minor things.
>
>First, is it please possible to add more descriptive comment into the
>code. Something like you put to the patch description, may be even a bit
>more descriptive. May be to the header of nand_bbt.c?
We usually don't have description in C code. Usually there is kernel doc
which describe the function and some description in Documentation/
which describes how should work. I could add something to the header of
the file and we move it leater to Doc*/mtd/bbt_hadling.txt ?

>The idea is that the BBT code is quite difficult to follow, and you make
>it even more complex, so a comment describing various flavors of BBT
>would be very useful.
Okay.

>And then several stylistic nitpicks.
>
>>  	while (totlen) {
>> +
>>  		len = min(totlen, (size_t) (1 << this->bbt_erase_shift));
>
>These extra whits spaces are not really needed. Ideally, your patch
>should make checkpatch.pl happy.
I can get rid of a few extra lines. checkpatch was happy with the patch
except with one >80 line which had 81.

>> +
>> +		if (marker_len) {
>> +			/*
>> +			 * In case the BBT marker is not in the OOB area it
>> +			 * will be just in the first page
>> +			 */
>
>Yes, this is the right multi-line comment. You may also put a dot at the
>end. But in other places you used not the right multi-line comments.
The missing dot is probally easy to fix. Other place where I did not use
correct multi-line comments are the .h file were I successfully
attempted to follow the style there. I'm going to fix it as well.

>
>> +		} else if (td->options & NAND_BBT_NO_OOB) {
>> +
>> +			ooboffs = 0;
>
>Extra blank line.
>
>> +			offs = td->len;
>> +			/* the version byte */
>> +			if (td->options & NAND_BBT_VERSION)
>> +				offs++;
>> +			/* Calc length */
>> +			len = (size_t) (numblocks >> sft);
>> +			len += offs;
>> +			/* Make it page aligned ! */
>> +			len = ALIGN(len, mtd->writesize);
>> +			/* Preset the buffer with 0xff */
>> +			memset(buf, 0xff, len);
>> +			/* Pattern is located at the begin of first page */
>> +			memcpy(buf, td->pattern, td->len);
>> +
>>  		} else {
>
>And here.
Both gone.

>
>> -/* Use a flash based bad block table. This option is passed to the
>> - * default bad block table function. */
>> +/* Use a flash based bad block table. OOB identifier is saved in OOB area.
>> + * This option is passed to the default bad block table function. */
>
>Would be nice to convert this to the right multi-line comment.
Will do in this patch.

>>  #define NAND_USE_FLASH_BBT	0x00010000
>>  /* This option skips the bbt scan during initialization. */
>>  #define NAND_SKIP_BBTSCAN	0x00020000
>> @@ -215,6 +215,9 @@ typedef enum {
>>  #define NAND_OWN_BUFFERS	0x00040000
>>  /* Chip may not exist, so silence any errors in scan */
>>  #define NAND_SCAN_SILENT_NODEV	0x00080000
>> +/* If passed additionally to NAND_USE_FLASH_BBT then BBT code will not touch
>> + * the OOB area */
>
>And this. Yeah, I know this file uses different styles, but you could
>convert even all of them.
I would prefer to send a cleanup afterwards and not mixing code changes
with no changes.
You want someone to keep checkpatch shut for other files as well or just
this one?

Sebastian

Re: [PATCH 3/5] mtd/nand: add support for BBT without OOB

[Artem Bityutskiy]

On Thu, 2010-09-30 at 10:51 +0200, Sebastian Andrzej Siewior wrote:
> * Artem Bityutskiy | 2010-09-29 23:11:04 [+0300]:
> 
> >Few minor things.
> >
> >First, is it please possible to add more descriptive comment into the
> >code. Something like you put to the patch description, may be even a bit
> >more descriptive. May be to the header of nand_bbt.c?
> We usually don't have description in C code. Usually there is kernel doc
> which describe the function and some description in Documentation/
> which describes how should work. I could add something to the header of
> the file and we move it leater to Doc*/mtd/bbt_hadling.txt ?

My point was that it is better to have a somewhat descriptive comment
in .c file which explains the differences in BBTs and reasons for them
and classifies them. This helps a lot. I mean, .c file comment is better
than no comments.

But a got Doc*/mtd/bbt_hadling.txt would be perfect, if you go that far,
this would be even better.

> >> +		if (marker_len) {
> >> +			/*
> >> +			 * In case the BBT marker is not in the OOB area it
> >> +			 * will be just in the first page
> >> +			 */
> >
> >Yes, this is the right multi-line comment. You may also put a dot at the
> >end. But in other places you used not the right multi-line comments.
> The missing dot is probally easy to fix. Other place where I did not use
> correct multi-line comments are the .h file were I successfully
> attempted to follow the style there. I'm going to fix it as well.

Thanks!

> >> +/* If passed additionally to NAND_USE_FLASH_BBT then BBT code will not touch
> >> + * the OOB area */
> >
> >And this. Yeah, I know this file uses different styles, but you could
> >convert even all of them.
> I would prefer to send a cleanup afterwards and not mixing code changes
> with no changes.

Ok.

> You want someone to keep checkpatch shut for other files as well or just
> this one?

You could do this for this file, but if you do for other files, this is
even better :-) Do as much extra contribution as you feel like!

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

[PATCH v2] mtd/nand: add support for BBT without OOB

[Sebastian Andrzej Siewior]

The first (sixt) byte in the OOB area contains vendor's bad block
information. During identification of the NAND chip this information is
collected by scanning the complete chip.
The option NAND_USE_FLASH_BBT is used to store this information in a sector so
we don't have to scan the complete flash. Unfortunately the code stores
a marker in order to recognize the BBT in the OOB area. This will fail
if the OOB area is completely used for ECC.
This patch introduces the option NAND_USE_FLASH_BBT_NO_OOB which has to be
used with NAND_USE_FLASH_BBT. It will then store BBT on flash without
touching the OOB area. The BBT format on flash remains same except the
first page starts with the recognition pattern followed by the version byte.
This change was tested in nandsim and it looks good so far :)

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mtd/nand/nand_bbt.c |  228 ++++++++++++++++++++++++++++++++++++++-----
 include/linux/mtd/bbm.h     |    2 +
 include/linux/mtd/nand.h    |   11 ++-
 3 files changed, 213 insertions(+), 28 deletions(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 587297e..b7b63cf 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -13,28 +13,37 @@
  * Description:
  *
  * When nand_scan_bbt is called, then it tries to find the bad block table
- * depending on the options in the bbt descriptor(s). If a bbt is found
- * then the contents are read and the memory based bbt is created. If a
- * mirrored bbt is selected then the mirror is searched too and the
- * versions are compared. If the mirror has a greater version number
- * than the mirror bbt is used to build the memory based bbt.
+ * depending on the options in the BBT descriptor(s). If no flash based BBT
+ * (NAND_USE_FLASH_BBT) is specified then the device is scanned for factory
+ * marked good / bad blocks. This information is used to create a memory BBT.
+ * Once a new bad block is discovered then the "factory" information is updated
+ * on the device.
+ * If a flash based BBT is specified then the function first tries to find the
+ * BBT on flash. If a BBT is found then the contents are read and the memory
+ * based BBT is created. If a mirrored BBT is selected then the mirror is
+ * searched too and the versions are compared. If the mirror has a greater
+ * version number than the mirror BBT is used to build the memory based BBT.
  * If the tables are not versioned, then we "or" the bad block information.
- * If one of the bbt's is out of date or does not exist it is (re)created.
- * If no bbt exists at all then the device is scanned for factory marked
+ * If one of the BBTs is out of date or does not exist it is (re)created.
+ * If no BBT exists at all then the device is scanned for factory marked
  * good / bad blocks and the bad block tables are created.
  *
- * For manufacturer created bbts like the one found on M-SYS DOC devices
- * the bbt is searched and read but never created
+ * For manufacturer created BBTs like the one found on M-SYS DOC devices
+ * the BBT is searched and read but never created
  *
- * The autogenerated bad block table is located in the last good blocks
+ * The auto generated bad block table is located in the last good blocks
  * of the device. The table is mirrored, so it can be updated eventually.
- * The table is marked in the oob area with an ident pattern and a version
- * number which indicates which of both tables is more up to date.
+ * The table is marked in the OOB area with an ident pattern and a version
+ * number which indicates which of both tables is more up to date. If the NAND
+ * controller needs the complete OOB area for the ECC information then the
+ * option NAND_USE_FLASH_BBT_NO_OOB should be used: it moves the ident pattern
+ * and the version byte into the data area and the OOB area will remain
+ * untouched.
  *
  * The table uses 2 bits per block
- * 11b: 	block is good
- * 00b: 	block is factory marked bad
- * 01b, 10b: 	block is marked bad due to wear
+ * 11b:		block is good
+ * 00b:		block is factory marked bad
+ * 01b, 10b:	block is marked bad due to wear
  *
  * The memory bad block table uses the following scheme:
  * 00b:		block is good
@@ -59,6 +68,16 @@
 #include <linux/delay.h>
 #include <linux/vmalloc.h>
 
+static int check_pattern_no_oob(uint8_t *buf, struct nand_bbt_descr *td)
+{
+	int ret;
+
+	ret = memcmp(buf, td->pattern, td->len);
+	if (!ret)
+		return ret;
+	return -1;
+}
+
 /**
  * check_pattern - [GENERIC] check if a pattern is in the buffer
  * @buf:	the buffer to search
@@ -77,6 +96,9 @@ static int check_pattern(uint8_t *buf, int len, int paglen, struct nand_bbt_desc
 	int i, end = 0;
 	uint8_t *p = buf;
 
+	if (td->options & NAND_BBT_NO_OOB)
+		return check_pattern_no_oob(buf, td);
+
 	end = paglen + td->offs;
 	if (td->options & NAND_BBT_SCANEMPTY) {
 		for (i = 0; i < end; i++) {
@@ -156,6 +178,25 @@ static int check_short_pattern(uint8_t *buf, struct nand_bbt_descr *td)
 }
 
 /**
+ * add_marker_len - compute the length of the marker in data area
+ * @td:		BBT descriptor used for computation
+ *
+ * The length will be 0 if the markeris located in OOB area.
+ */
+static u32 add_marker_len(struct nand_bbt_descr *td)
+{
+	u32 len;
+
+	if (!(td->options & NAND_BBT_NO_OOB))
+		return 0;
+
+	len = td->len;
+	if (td->options & NAND_BBT_VERSION)
+		len++;
+	return len;
+}
+
+/**
  * read_bbt - [GENERIC] Read the bad block table starting from page
  * @mtd:	MTD device structure
  * @buf:	temporary buffer
@@ -176,13 +217,24 @@ static int read_bbt(struct mtd_info *mtd, uint8_t *buf, int page, int num,
 	loff_t from;
 	int bits = td->options & NAND_BBT_NRBITS_MSK;
 	uint8_t msk = (uint8_t) ((1 << bits) - 1);
+	u32 marker_len;
 	int reserved_block_code = td->reserved_block_code;
 
 	totlen = (num * bits) >> 3;
+	marker_len = add_marker_len(td);
 	from = ((loff_t) page) << this->page_shift;
 
 	while (totlen) {
 		len = min(totlen, (size_t) (1 << this->bbt_erase_shift));
+		if (marker_len) {
+			/*
+			 * In case the BBT marker is not in the OOB area it
+			 * will be just in the first page.
+			 */
+			len -= marker_len;
+			from += marker_len;
+			marker_len = 0;
+		}
 		res = mtd->read(mtd, from, len, &retlen, buf);
 		if (res < 0) {
 			if (retlen != len) {
@@ -261,9 +313,25 @@ static int read_abs_bbt(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_desc
 }
 
 /*
+ * BBT marker is in the first page, no OOB.
+ */
+static int scan_read_raw_data(struct mtd_info *mtd, uint8_t *buf, loff_t offs,
+			 struct nand_bbt_descr *td)
+{
+	size_t retlen;
+	size_t len;
+
+	len = td->len;
+	if (td->options & NAND_BBT_VERSION)
+		len++;
+
+	return mtd->read(mtd, offs, len, &retlen, buf);
+}
+
+/*
  * Scan read raw data from flash
  */
-static int scan_read_raw(struct mtd_info *mtd, uint8_t *buf, loff_t offs,
+static int scan_read_raw_oob(struct mtd_info *mtd, uint8_t *buf, loff_t offs,
 			 size_t len)
 {
 	struct mtd_oob_ops ops;
@@ -296,6 +364,15 @@ static int scan_read_raw(struct mtd_info *mtd, uint8_t *buf, loff_t offs,
 	return 0;
 }
 
+static int scan_read_raw(struct mtd_info *mtd, uint8_t *buf, loff_t offs,
+			 size_t len, struct nand_bbt_descr *td)
+{
+	if (td->options & NAND_BBT_NO_OOB)
+		return scan_read_raw_data(mtd, buf, offs, td);
+	else
+		return scan_read_raw_oob(mtd, buf, offs, len);
+}
+
 /*
  * Scan write data with oob to flash
  */
@@ -314,6 +391,15 @@ static int scan_write_bbt(struct mtd_info *mtd, loff_t offs, size_t len,
 	return mtd->write_oob(mtd, offs, &ops);
 }
 
+static u32 bbt_get_ver_offs(struct mtd_info *mtd, struct nand_bbt_descr *td)
+{
+	u32 ver_offs = td->veroffs;
+
+	if (!(td->options & NAND_BBT_NO_OOB))
+		ver_offs += mtd->writesize;
+	return ver_offs;
+}
+
 /**
  * read_abs_bbts - [GENERIC] Read the bad block table(s) for all chips starting at a given page
  * @mtd:	MTD device structure
@@ -333,8 +419,8 @@ static int read_abs_bbts(struct mtd_info *mtd, uint8_t *buf,
 	/* Read the primary version, if available */
 	if (td->options & NAND_BBT_VERSION) {
 		scan_read_raw(mtd, buf, (loff_t)td->pages[0] << this->page_shift,
-			      mtd->writesize);
-		td->version[0] = buf[mtd->writesize + td->veroffs];
+			      mtd->writesize, td);
+		td->version[0] = buf[bbt_get_ver_offs(mtd, td)];
 		printk(KERN_DEBUG "Bad block table at page %d, version 0x%02X\n",
 		       td->pages[0], td->version[0]);
 	}
@@ -342,8 +428,8 @@ static int read_abs_bbts(struct mtd_info *mtd, uint8_t *buf,
 	/* Read the mirror version, if available */
 	if (md && (md->options & NAND_BBT_VERSION)) {
 		scan_read_raw(mtd, buf, (loff_t)md->pages[0] << this->page_shift,
-			      mtd->writesize);
-		md->version[0] = buf[mtd->writesize + md->veroffs];
+			      mtd->writesize, td);
+		md->version[0] = buf[bbt_get_ver_offs(mtd, md)];
 		printk(KERN_DEBUG "Bad block table at page %d, version 0x%02X\n",
 		       md->pages[0], md->version[0]);
 	}
@@ -359,7 +445,7 @@ static int scan_block_full(struct mtd_info *mtd, struct nand_bbt_descr *bd,
 {
 	int ret, j;
 
-	ret = scan_read_raw(mtd, buf, offs, readlen);
+	ret = scan_read_raw_oob(mtd, buf, offs, readlen);
 	if (ret)
 		return ret;
 
@@ -466,6 +552,8 @@ static int create_bbt(struct mtd_info *mtd, uint8_t *buf,
 	for (i = startblock; i < numblocks;) {
 		int ret;
 
+		BUG_ON(bd->options & NAND_BBT_NO_OOB);
+
 		if (bd->options & NAND_BBT_SCANALLPAGES)
 			ret = scan_block_full(mtd, bd, from, buf, readlen,
 					      scanlen, len);
@@ -547,11 +635,12 @@ static int search_bbt(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_descr
 			loff_t offs = (loff_t)actblock << this->bbt_erase_shift;
 
 			/* Read first page */
-			scan_read_raw(mtd, buf, offs, mtd->writesize);
+			scan_read_raw(mtd, buf, offs, mtd->writesize, td);
 			if (!check_pattern(buf, scanlen, mtd->writesize, td)) {
 				td->pages[i] = actblock << blocktopage;
 				if (td->options & NAND_BBT_VERSION) {
-					td->version[i] = buf[mtd->writesize + td->veroffs];
+					offs = bbt_get_ver_offs(mtd, td);
+					td->version[i] = buf[offs];
 				}
 				break;
 			}
@@ -735,6 +824,21 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
 			memset(&buf[offs], 0xff, (size_t) (numblocks >> sft));
 			ooboffs = len + (pageoffs * mtd->oobsize);
 
+		} else if (td->options & NAND_BBT_NO_OOB) {
+			ooboffs = 0;
+			offs = td->len;
+			/* the version byte */
+			if (td->options & NAND_BBT_VERSION)
+				offs++;
+			/* Calc length */
+			len = (size_t) (numblocks >> sft);
+			len += offs;
+			/* Make it page aligned ! */
+			len = ALIGN(len, mtd->writesize);
+			/* Preset the buffer with 0xff */
+			memset(buf, 0xff, len);
+			/* Pattern is located at the begin of first page */
+			memcpy(buf, td->pattern, td->len);
 		} else {
 			/* Calc length */
 			len = (size_t) (numblocks >> sft);
@@ -773,7 +877,9 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
 		if (res < 0)
 			goto outerr;
 
-		res = scan_write_bbt(mtd, to, len, buf, &buf[len]);
+		res = scan_write_bbt(mtd, to, len, buf,
+				td->options & NAND_BBT_NO_OOB ? NULL :
+				&buf[len]);
 		if (res < 0)
 			goto outerr;
 
@@ -984,6 +1090,49 @@ static void mark_bbt_region(struct mtd_info *mtd, struct nand_bbt_descr *td)
 }
 
 /**
+ * verify_bbt_descr - verify the bad block description
+ * @bd:			the table to verify
+ *
+ * This functions performs a few sanity checks on the bad block description
+ * table.
+ */
+static void verify_bbt_descr(struct mtd_info *mtd, struct nand_bbt_descr *bd)
+{
+	struct nand_chip *this = mtd->priv;
+	u32 pattern_len = bd->len;
+	u32 bits = bd->options & NAND_BBT_NRBITS_MSK;
+	u32 table_size;
+
+	if (!bd)
+		return;
+	BUG_ON((this->options & NAND_USE_FLASH_BBT_NO_OOB) &&
+			!(this->options & NAND_USE_FLASH_BBT));
+	BUG_ON(!bits);
+
+	if (bd->options & NAND_BBT_VERSION)
+		pattern_len++;
+
+	if (bd->options & NAND_BBT_NO_OOB) {
+		BUG_ON(!(this->options & NAND_USE_FLASH_BBT));
+		BUG_ON(!(this->options & NAND_USE_FLASH_BBT_NO_OOB));
+		BUG_ON(bd->offs);
+		if (bd->options & NAND_BBT_VERSION)
+			BUG_ON(bd->veroffs != bd->len);
+		BUG_ON(bd->options & NAND_BBT_SAVECONTENT);
+	}
+
+	if (bd->options & NAND_BBT_PERCHIP)
+		table_size = this->chipsize >> this->bbt_erase_shift;
+	else
+		table_size = mtd->size >> this->bbt_erase_shift;
+	table_size >>= 3;
+	table_size *= bits;
+	if (bd->options & NAND_BBT_NO_OOB)
+		table_size += pattern_len;
+	BUG_ON(table_size > (1 << this->bbt_erase_shift));
+}
+
+/**
  * nand_scan_bbt - [NAND Interface] scan, find, read and maybe create bad block table(s)
  * @mtd:	MTD device structure
  * @bd:		descriptor for the good/bad block search pattern
@@ -1024,6 +1173,8 @@ int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
 		}
 		return res;
 	}
+	verify_bbt_descr(mtd, td);
+	verify_bbt_descr(mtd, md);
 
 	/* Allocate a temporary buffer for one eraseblock incl. oob */
 	len = (1 << this->bbt_erase_shift);
@@ -1167,6 +1318,26 @@ static struct nand_bbt_descr bbt_mirror_descr = {
 	.pattern = mirror_pattern
 };
 
+static struct nand_bbt_descr bbt_main_no_bbt_descr = {
+	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
+		| NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP
+		| NAND_BBT_NO_OOB,
+	.len = 4,
+	.veroffs = 4,
+	.maxblocks = 4,
+	.pattern = bbt_pattern
+};
+
+static struct nand_bbt_descr bbt_mirror_no_bbt_descr = {
+	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
+		| NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP
+		| NAND_BBT_NO_OOB,
+	.len = 4,
+	.veroffs = 4,
+	.maxblocks = 4,
+	.pattern = mirror_pattern
+};
+
 #define BBT_SCAN_OPTIONS (NAND_BBT_SCANLASTPAGE | NAND_BBT_SCAN2NDPAGE | \
 		NAND_BBT_SCANBYTE1AND6)
 /**
@@ -1237,8 +1408,13 @@ int nand_default_bbt(struct mtd_info *mtd)
 	if (this->options & NAND_USE_FLASH_BBT) {
 		/* Use the default pattern descriptors */
 		if (!this->bbt_td) {
-			this->bbt_td = &bbt_main_descr;
-			this->bbt_md = &bbt_mirror_descr;
+			if (this->options & NAND_USE_FLASH_BBT_NO_OOB) {
+				this->bbt_td = &bbt_main_no_bbt_descr;
+				this->bbt_md = &bbt_mirror_no_bbt_descr;
+			} else {
+				this->bbt_td = &bbt_main_descr;
+				this->bbt_md = &bbt_mirror_descr;
+			}
 		}
 		if (!this->badblock_pattern) {
 			this->badblock_pattern = (mtd->writesize > 512) ? &largepage_flashbased : &smallpage_flashbased;
diff --git a/include/linux/mtd/bbm.h b/include/linux/mtd/bbm.h
index 7fa20be..ba15b52 100644
--- a/include/linux/mtd/bbm.h
+++ b/include/linux/mtd/bbm.h
@@ -102,6 +102,8 @@ struct nand_bbt_descr {
 #define NAND_BBT_SCANBYTE1AND6 0x00100000
 /* The nand_bbt_descr was created dynamicaly and must be freed */
 #define NAND_BBT_DYNAMICSTRUCT 0x00200000
+/* The bad block table does not OOB for marker */
+#define NAND_BBT_NO_OOB		0x00400000
 
 /* The maximum number of blocks to scan for a bbt */
 #define NAND_BBT_SCAN_MAXBLOCKS	4
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 0f74454..27e3d0a 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -206,8 +206,10 @@ typedef enum {
 #define NAND_CHIPOPTIONS_MSK	(0x0000ffff & ~NAND_NO_AUTOINCR)
 
 /* Non chip related options */
-/* Use a flash based bad block table. This option is passed to the
- * default bad block table function. */
+/*
+ * Use a flash based bad block table. OOB identifier is saved in OOB area.
+ * This option is passed to the default bad block table function.
+ */
 #define NAND_USE_FLASH_BBT	0x00010000
 /* This option skips the bbt scan during initialization. */
 #define NAND_SKIP_BBTSCAN	0x00020000
@@ -216,6 +218,11 @@ typedef enum {
 #define NAND_OWN_BUFFERS	0x00040000
 /* Chip may not exist, so silence any errors in scan */
 #define NAND_SCAN_SILENT_NODEV	0x00080000
+/*
+ * If passed additionally to NAND_USE_FLASH_BBT then BBT code will not touch
+ * the OOB area.
+ */
+#define NAND_USE_FLASH_BBT_NO_OOB	0x00100000
 
 /* Options set by nand scan */
 /* Nand scan has allocated controller struct */
-- 
1.7.2.3

Re: [PATCH v2] mtd/nand: add support for BBT without OOB

[Artem Bityutskiy]

On Thu, 2010-09-30 at 21:28 +0200, Sebastian Andrzej Siewior wrote:
> The first (sixt) byte in the OOB area contains vendor's bad block
> information. During identification of the NAND chip this information is
> collected by scanning the complete chip.
> The option NAND_USE_FLASH_BBT is used to store this information in a sector so
> we don't have to scan the complete flash. Unfortunately the code stores
> a marker in order to recognize the BBT in the OOB area. This will fail
> if the OOB area is completely used for ECC.
> This patch introduces the option NAND_USE_FLASH_BBT_NO_OOB which has to be
> used with NAND_USE_FLASH_BBT. It will then store BBT on flash without
> touching the OOB area. The BBT format on flash remains same except the
> first page starts with the recognition pattern followed by the version byte.
> This change was tested in nandsim and it looks good so far :)
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Pushed to l2-mtd-2.6.git, thanks.

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

[PATCH 4/5] mtd/nand: introduce NAND_CREATE_EMPTY_BBT

[Sebastian Andrzej Siewior]

it will create an empty BBT table without considering vendor's BBT
information. Vendor's information may be unavailable if the NAND
controller has a different DATA & OOB layout or if this information is
allready purged.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mtd/nand/nand_bbt.c |    3 ++-
 include/linux/mtd/bbm.h     |    2 +-
 include/linux/mtd/nand.h    |    2 ++
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 11e4204..fd3295e 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -994,7 +994,8 @@ static int check_create(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_desc
 			continue;
 
 		/* Create the table in memory by scanning the chip(s) */
-		create_bbt(mtd, buf, bd, chipsel);
+		if (!(this->options & NAND_CREATE_EMPTY_BBT))
+			create_bbt(mtd, buf, bd, chipsel);
 
 		td->version[i] = 1;
 		if (md)
diff --git a/include/linux/mtd/bbm.h b/include/linux/mtd/bbm.h
index ba15b52..57cc0e6 100644
--- a/include/linux/mtd/bbm.h
+++ b/include/linux/mtd/bbm.h
@@ -84,7 +84,7 @@ struct nand_bbt_descr {
 #define NAND_BBT_PERCHIP	0x00000080
 /* bbt has a version counter at offset veroffs */
 #define NAND_BBT_VERSION	0x00000100
-/* Create a bbt if none axists */
+/* Create a bbt if none exists */
 #define NAND_BBT_CREATE		0x00000200
 /* Search good / bad pattern through all pages of a block */
 #define NAND_BBT_SCANALLPAGES	0x00000400
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 68caf20..c4b1055 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -218,6 +218,8 @@ typedef enum {
 /* If passed additionally to NAND_USE_FLASH_BBT then BBT code will not touch
  * the OOB area */
 #define NAND_USE_FLASH_BBT_NO_OOB	0x00100000
+/* Create an empty BBT with no vendor information if the BBT is available */
+#define NAND_CREATE_EMPTY_BBT		0x00200000
 
 /* Options set by nand scan */
 /* Nand scan has allocated controller struct */
-- 
1.7.2.3

Re: [PATCH 4/5] mtd/nand: introduce NAND_CREATE_EMPTY_BBT

[Artem Bityutskiy]

On Wed, 2010-09-29 at 19:43 +0200, Sebastian Andrzej Siewior wrote:
> it will create an empty BBT table without considering vendor's BBT
> information. Vendor's information may be unavailable if the NAND
> controller has a different DATA & OOB layout or if this information is
> allready purged.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

This one does not apply on top of your patch 3/5 v2, could you please
refresh it and resend?

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

[PATCH v2] mtd/nand: introduce NAND_CREATE_EMPTY_BBT

[Sebastian Andrzej Siewior]

it will create an empty BBT table without considering vendor's BBT
information. Vendor's information may be unavailable if the NAND
controller has a different DATA & OOB layout or this information may be
allready purged.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mtd/nand/nand_bbt.c |    3 ++-
 include/linux/mtd/bbm.h     |    2 +-
 include/linux/mtd/nand.h    |    2 ++
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index b7b63cf..a27984c 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -999,7 +999,8 @@ static int check_create(struct mtd_info *mtd, uint8_t *buf, struct nand_bbt_desc
 			continue;
 
 		/* Create the table in memory by scanning the chip(s) */
-		create_bbt(mtd, buf, bd, chipsel);
+		if (!(this->options & NAND_CREATE_EMPTY_BBT))
+			create_bbt(mtd, buf, bd, chipsel);
 
 		td->version[i] = 1;
 		if (md)
diff --git a/include/linux/mtd/bbm.h b/include/linux/mtd/bbm.h
index ba15b52..57cc0e6 100644
--- a/include/linux/mtd/bbm.h
+++ b/include/linux/mtd/bbm.h
@@ -84,7 +84,7 @@ struct nand_bbt_descr {
 #define NAND_BBT_PERCHIP	0x00000080
 /* bbt has a version counter at offset veroffs */
 #define NAND_BBT_VERSION	0x00000100
-/* Create a bbt if none axists */
+/* Create a bbt if none exists */
 #define NAND_BBT_CREATE		0x00000200
 /* Search good / bad pattern through all pages of a block */
 #define NAND_BBT_SCANALLPAGES	0x00000400
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 27e3d0a..d34b912 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -223,6 +223,8 @@ typedef enum {
  * the OOB area
  */
 #define NAND_USE_FLASH_BBT_NO_OOB	0x00100000
+/* Create an empty BBT with no vendor information if the BBT is available */
+#define NAND_CREATE_EMPTY_BBT		0x00200000
 
 /* Options set by nand scan */
 /* Nand scan has allocated controller struct */
-- 
1.7.2.3

Re: [PATCH v2] mtd/nand: introduce NAND_CREATE_EMPTY_BBT

[Artem Bityutskiy]

On Fri, 2010-10-01 at 21:37 +0200, Sebastian Andrzej Siewior wrote:
> it will create an empty BBT table without considering vendor's BBT
> information. Vendor's information may be unavailable if the NAND
> controller has a different DATA & OOB layout or this information may be
> allready purged.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Pushed to l2-mtd-2.6.git, and the nandsim patch as well.

git.infradead.org is down now for some reasons, but when it is up, I'll
push my tree out.

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

[PATCH 5/5] mtd/nandsim: add module param for BBT handling

[Sebastian Andrzej Siewior]

I used this to check the BBT on flash together with a hack in mtdchar in
order to read bad blocks.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/mtd/nand/nandsim.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
index c25648b..a6a73aa 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -107,6 +107,7 @@ static char *gravepages = NULL;
 static unsigned int rptwear = 0;
 static unsigned int overridesize = 0;
 static char *cache_file = NULL;
+static unsigned int bbt;
 
 module_param(first_id_byte,  uint, 0400);
 module_param(second_id_byte, uint, 0400);
@@ -130,6 +131,7 @@ module_param(gravepages,     charp, 0400);
 module_param(rptwear,        uint, 0400);
 module_param(overridesize,   uint, 0400);
 module_param(cache_file,     charp, 0400);
+module_param(bbt,	     uint, 0400);
 
 MODULE_PARM_DESC(first_id_byte,  "The first byte returned by NAND Flash 'read ID' command (manufacturer ID)");
 MODULE_PARM_DESC(second_id_byte, "The second byte returned by NAND Flash 'read ID' command (chip ID)");
@@ -162,6 +164,7 @@ MODULE_PARM_DESC(overridesize,   "Specifies the NAND Flash size overriding the I
 				 "The size is specified in erase blocks and as the exponent of a power of two"
 				 " e.g. 5 means a size of 32 erase blocks");
 MODULE_PARM_DESC(cache_file,     "File to use to cache nand pages instead of memory");
+MODULE_PARM_DESC(bbt,		 "0 OOB, 1 BBT with marker in OOB, 2 BBT with marker in data area");
 
 /* The largest possible page size */
 #define NS_LARGEST_PAGE_SIZE	4096
@@ -2264,6 +2267,18 @@ static int __init ns_init_module(void)
 	/* and 'badblocks' parameters to work */
 	chip->options   |= NAND_SKIP_BBTSCAN;
 
+	switch (bbt) {
+	case 2:
+		 chip->options |= NAND_USE_FLASH_BBT_NO_OOB;
+	case 1:
+		 chip->options |= NAND_USE_FLASH_BBT;
+	case 0:
+		break;
+	default:
+		NS_ERR("bbt has to be 0..2\n");
+		retval = -EINVAL;
+		goto error;
+	}
 	/*
 	 * Perform minimum nandsim structure initialization to handle
 	 * the initial ID read command correctly
@@ -2321,10 +2336,10 @@ static int __init ns_init_module(void)
 	if ((retval = init_nandsim(nsmtd)) != 0)
 		goto err_exit;
 
-	if ((retval = parse_badblocks(nand, nsmtd)) != 0)
+	if ((retval = nand_default_bbt(nsmtd)) != 0)
 		goto err_exit;
 
-	if ((retval = nand_default_bbt(nsmtd)) != 0)
+	if ((retval = parse_badblocks(nand, nsmtd)) != 0)
 		goto err_exit;
 
 	/* Register NAND partitions */
-- 
1.7.2.3

Re: Add support for BBT without touching OOB area

[Artem Bityutskiy]

On Wed, 2010-09-29 at 19:43 +0200, Sebastian Andrzej Siewior wrote:
> Without this patch (number three) NAND controllers which use complete OOB
> area for ECC information can not use a BBT at all. The vendor bit is
> gone after the first write and the flash BBT stores a marker in the OOB
> area. The patch simply moves the marker into the data area.
> 
> The series contains:
> - [PATCH 1/5] mtd/nand: use ALIGN where possible
> - [PATCH 2/5] mtd/nand: pull in td into read_bbt()
> clean up.

Pushed these to l2-mtd-2.6.git.

> - [PATCH 3/5] mtd/nand: add support for BBT without OOB
> adds the functionality.

Sent some minor requests.

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)