[PATCH 0/5] mtd: nand: misc BBT fixes

[PATCH 0/5] mtd: nand: misc BBT fixes

[Brian Norris]

A few assorted changes. Some of these are geared toward refactoring the BBT
code for being reused for SPI NAND.

If possible, I'd really like to get some testing for the diskonchip changes.
It's an old driver, but it appears as if some still use it. Perhaps Alexander
(CC'd) can report?

Brian Norris (5):
  mtd: nand_bbt: drop unnecessary header
  mtd: diskonchip: don't call nand_scan_bbt() directly
  mtd: nand_bbt: make nand_scan_bbt() static
  mtd: nand_bbt: unify/fix error handling in nand_scan_bbt()
  mtd: nand_bbt: fix theoretical integer overflow in BBT write

 drivers/mtd/nand/diskonchip.c | 27 ++++++++++++++++-----------
 drivers/mtd/nand/nand_bbt.c   | 24 ++++++++++++++----------
 include/linux/mtd/nand.h      |  1 -
 3 files changed, 30 insertions(+), 22 deletions(-)

-- 
2.1.0

[PATCH 1/5] mtd: nand_bbt: drop unnecessary header

[Brian Norris]

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_bbt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 9bb8453d224e..307a285afb78 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -64,7 +64,6 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/bbm.h>
 #include <linux/mtd/nand.h>
-#include <linux/mtd/nand_ecc.h>
 #include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/vmalloc.h>
-- 
2.1.0

[PATCH 2/5] mtd: diskonchip: don't call nand_scan_bbt() directly

[Brian Norris]

The diskonchip driver almost uses the default nand_base hooks as-is,
except that it provides custom on-flash BBT descriptors and avoids using
factory-marked bad blockers.

So let's refactor the BBT initialization code into a private 'late_init'
hook which handles all the private details. Note the usage of
NAND_SKIP_BBTSCAN, which allows us to defer the BBT scan until we've
prepared everything.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/diskonchip.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/diskonchip.c b/drivers/mtd/nand/diskonchip.c
index f68a7bccecdc..e5800146cf33 100644
--- a/drivers/mtd/nand/diskonchip.c
+++ b/drivers/mtd/nand/diskonchip.c
@@ -69,6 +69,9 @@ struct doc_priv {
 	int mh0_page;
 	int mh1_page;
 	struct mtd_info *nextdoc;
+
+	/* Handle the last stage of initialization (BBT scan, partitioning) */
+	int (*late_init)(struct mtd_info *mtd);
 };
 
 /* This is the syndrome computed by the HW ecc generator upon reading an empty
@@ -1294,10 +1297,10 @@ static int __init nftl_scan_bbt(struct mtd_info *mtd)
 		this->bbt_md = NULL;
 	}
 
-	/* It's safe to set bd=NULL below because NAND_BBT_CREATE is not set.
-	   At least as nand_bbt.c is currently written. */
-	if ((ret = nand_scan_bbt(mtd, NULL)))
+	ret = this->scan_bbt(mtd);
+	if (ret)
 		return ret;
+
 	mtd_device_register(mtd, NULL, 0);
 	if (!no_autopart)
 		mtd_device_register(mtd, parts, numparts);
@@ -1344,10 +1347,10 @@ static int __init inftl_scan_bbt(struct mtd_info *mtd)
 		this->bbt_md->pattern = "TBB_SYSM";
 	}
 
-	/* It's safe to set bd=NULL below because NAND_BBT_CREATE is not set.
-	   At least as nand_bbt.c is currently written. */
-	if ((ret = nand_scan_bbt(mtd, NULL)))
+	ret = this->scan_bbt(mtd);
+	if (ret)
 		return ret;
+
 	memset((char *)parts, 0, sizeof(parts));
 	numparts = inftl_partscan(mtd, parts);
 	/* At least for now, require the INFTL Media Header.  We could probably
@@ -1369,7 +1372,7 @@ static inline int __init doc2000_init(struct mtd_info *mtd)
 	this->read_byte = doc2000_read_byte;
 	this->write_buf = doc2000_writebuf;
 	this->read_buf = doc2000_readbuf;
-	this->scan_bbt = nftl_scan_bbt;
+	doc->late_init = nftl_scan_bbt;
 
 	doc->CDSNControl = CDSN_CTRL_FLASH_IO | CDSN_CTRL_ECC_IO;
 	doc2000_count_chips(mtd);
@@ -1396,13 +1399,13 @@ static inline int __init doc2001_init(struct mtd_info *mtd)
 		   can have multiple chips. */
 		doc2000_count_chips(mtd);
 		mtd->name = "DiskOnChip 2000 (INFTL Model)";
-		this->scan_bbt = inftl_scan_bbt;
+		doc->late_init = inftl_scan_bbt;
 		return (4 * doc->chips_per_floor);
 	} else {
 		/* Bog-standard Millennium */
 		doc->chips_per_floor = 1;
 		mtd->name = "DiskOnChip Millennium";
-		this->scan_bbt = nftl_scan_bbt;
+		doc->late_init = nftl_scan_bbt;
 		return 1;
 	}
 }
@@ -1415,7 +1418,7 @@ static inline int __init doc2001plus_init(struct mtd_info *mtd)
 	this->read_byte = doc2001plus_read_byte;
 	this->write_buf = doc2001plus_writebuf;
 	this->read_buf = doc2001plus_readbuf;
-	this->scan_bbt = inftl_scan_bbt;
+	doc->late_init = inftl_scan_bbt;
 	this->cmd_ctrl = NULL;
 	this->select_chip = doc2001plus_select_chip;
 	this->cmdfunc = doc2001plus_command;
@@ -1591,6 +1594,8 @@ static int __init doc_probe(unsigned long physadr)
 	nand->ecc.bytes		= 6;
 	nand->ecc.strength	= 2;
 	nand->bbt_options	= NAND_BBT_USE_FLASH;
+	/* Skip the automatic BBT scan so we can run it manually */
+	nand->options		|= NAND_SKIP_BBTSCAN;
 
 	doc->physadr		= physadr;
 	doc->virtadr		= virtadr;
@@ -1608,7 +1613,7 @@ static int __init doc_probe(unsigned long physadr)
 	else
 		numchips = doc2001_init(mtd);
 
-	if ((ret = nand_scan(mtd, numchips))) {
+	if ((ret = nand_scan(mtd, numchips)) || (ret = doc->late_init(mtd))) {
 		/* DBB note: i believe nand_release is necessary here, as
 		   buffers may have been allocated in nand_base.  Check with
 		   Thomas. FIX ME! */
-- 
2.1.0

[PATCH 3/5] mtd: nand_bbt: make nand_scan_bbt() static

[Brian Norris]

This implementation detail is no longer needed outside of nand_bbt.c.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_bbt.c | 2 +-
 include/linux/mtd/nand.h    | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 307a285afb78..53e17586fbed 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -1074,7 +1074,7 @@ static void verify_bbt_descr(struct mtd_info *mtd, struct nand_bbt_descr *bd)
  * The bad block table memory is allocated here. It must be freed by calling
  * the nand_free_bbt function.
  */
-int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
+static int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
 {
 	struct nand_chip *this = mtd->priv;
 	int len, res = 0;
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 3d4ea7eb2b68..6c51876941f3 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -833,7 +833,6 @@ struct nand_manufacturers {
 extern struct nand_flash_dev nand_flash_ids[];
 extern struct nand_manufacturers nand_manuf_ids[];
 
-extern int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd);
 extern int nand_default_bbt(struct mtd_info *mtd);
 extern int nand_markbad_bbt(struct mtd_info *mtd, loff_t offs);
 extern int nand_isreserved_bbt(struct mtd_info *mtd, loff_t offs);
-- 
2.1.0

[PATCH 4/5] mtd: nand_bbt: unify/fix error handling in nand_scan_bbt()

[Brian Norris]

Don't leak this->bbt, and return early if check_create() fails. It helps
to have a single error path to avoid these problems.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_bbt.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 53e17586fbed..516db2c4524b 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -1077,7 +1077,7 @@ static void verify_bbt_descr(struct mtd_info *mtd, struct nand_bbt_descr *bd)
 static int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
 {
 	struct nand_chip *this = mtd->priv;
-	int len, res = 0;
+	int len, res;
 	uint8_t *buf;
 	struct nand_bbt_descr *td = this->bbt_td;
 	struct nand_bbt_descr *md = this->bbt_md;
@@ -1098,10 +1098,9 @@ static int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
 	if (!td) {
 		if ((res = nand_memory_bbt(mtd, bd))) {
 			pr_err("nand_bbt: can't scan flash and build the RAM-based BBT\n");
-			kfree(this->bbt);
-			this->bbt = NULL;
+			goto err;
 		}
-		return res;
+		return 0;
 	}
 	verify_bbt_descr(mtd, td);
 	verify_bbt_descr(mtd, md);
@@ -1111,9 +1110,8 @@ static int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
 	len += (len >> this->page_shift) * mtd->oobsize;
 	buf = vmalloc(len);
 	if (!buf) {
-		kfree(this->bbt);
-		this->bbt = NULL;
-		return -ENOMEM;
+		res = -ENOMEM;
+		goto err;
 	}
 
 	/* Is the bbt at a given page? */
@@ -1125,6 +1123,8 @@ static int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
 	}
 
 	res = check_create(mtd, buf, bd);
+	if (res)
+		goto err;
 
 	/* Prevent the bbt regions from erasing / writing */
 	mark_bbt_region(mtd, td);
@@ -1132,6 +1132,11 @@ static int nand_scan_bbt(struct mtd_info *mtd, struct nand_bbt_descr *bd)
 		mark_bbt_region(mtd, md);
 
 	vfree(buf);
+	return 0;
+
+err:
+	kfree(this->bbt);
+	this->bbt = NULL;
 	return res;
 }
 
-- 
2.1.0

[PATCH 5/5] mtd: nand_bbt: fix theoretical integer overflow in BBT write

[Brian Norris]

This statement was written with a cast-to-loff_t to be sure to have a
full 64-bit mask. However, we don't account for the fact that
'1 << this->bbt_erase_shift' might already overflow.

This will not be a problem in practice, since eraseblocks should never
be anywhere near 4GiB. But we can do this for completeness, and quiet
Coverity in the meantime. CID #1226806.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_bbt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 516db2c4524b..2c4fa1a17031 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -719,7 +719,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf,
 		/* Must we save the block contents? */
 		if (td->options & NAND_BBT_SAVECONTENT) {
 			/* Make it block aligned */
-			to &= ~((loff_t)((1 << this->bbt_erase_shift) - 1));
+			to &= ~(((loff_t)1 << this->bbt_erase_shift) - 1);
 			len = 1 << this->bbt_erase_shift;
 			res = mtd_read(mtd, to, len, &retlen, buf);
 			if (res < 0) {
-- 
2.1.0

Re: [PATCH 0/5] mtd: nand: misc BBT fixes

[Brian Norris]

On Sat, Feb 28, 2015 at 02:13:08AM -0800, Brian Norris wrote:
> A few assorted changes. Some of these are geared toward refactoring the BBT
> code for being reused for SPI NAND.
> 
> If possible, I'd really like to get some testing for the diskonchip changes.
> It's an old driver, but it appears as if some still use it. Perhaps Alexander
> (CC'd) can report?

Pushed all 5 to l2-mtd.git, since the only comments I got were reposts
of the patches...

Brian