[PATCH] MTD: OneNAND: fix numerous races

[Artem Bityutskiy <dedekind@infradead.org>]

>From 5383a93bc534aa15c4e2a260fe5ca4a4ecb8310e Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Date: Fri, 21 Sep 2007 15:34:43 +0300
Subject: [PATCH] MTD: OneNAND: fix numerous races

This patch make the OneNAND driver much less racy. It fixes
our "onenand_wait: read timeout!" heisenbugs. The reason of
these bugs was that the driver did not lock the chip when
accessing OTP, and it screwed up OneNAND state when the OTP
was read while JFFS2 was doing FS checking.

This patch also fixes other races I spotted:
1. BBT was not protected
2. Access to ecc_stats was not protected

Now the chip is locked when BBT is accessed.

To fix all of these I basically split all interface functions
on 'function()' and 'function_nolock()' parts.

This was tested on N800 hardware.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 drivers/mtd/onenand/onenand_base.c |  212 +++++++++++++++++++----------------
 1 files changed, 115 insertions(+), 97 deletions(-)

diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 4e1de12..8d97df0 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -718,18 +718,8 @@ static void onenand_release_device(struct mtd_info *mtd)
 	spin_unlock(&this->chip_lock);
 }
 
-/**
- * onenand_read - [MTD Interface] Read data from flash
- * @param mtd		MTD device structure
- * @param from		offset to read from
- * @param len		number of bytes to read
- * @param retlen	pointer to variable to store the number of read bytes
- * @param buf		the databuffer to put data
- *
- * Read with ecc
-*/
-static int onenand_read(struct mtd_info *mtd, loff_t from, size_t len,
-	size_t *retlen, u_char *buf)
+static int onenand_read_nolock(struct mtd_info *mtd, loff_t from, size_t len,
+			       size_t *retlen, u_char *buf)
 {
 	struct onenand_chip *this = mtd->priv;
 	struct mtd_ecc_stats stats;
@@ -737,18 +727,16 @@ static int onenand_read(struct mtd_info *mtd, loff_t from, size_t len,
 	int thislen;
 	int ret = 0, boundary = 0;
 
-	DEBUG(MTD_DEBUG_LEVEL3, "onenand_read: from = 0x%08x, len = %i\n", (unsigned int) from, (int) len);
+	DEBUG(MTD_DEBUG_LEVEL3, "onenand_read_nolock: from = 0x%08x, ""len = %i\n",
+	      (unsigned int) from, (int) len);
 
 	/* Do not allow reads past end of device */
 	if ((from + len) > mtd->size) {
-		printk(KERN_ERR "onenand_read: Attempt read beyond end of device\n");
+		printk(KERN_ERR "onenand_read_nolock: Attempt read beyond end of device\n");
 		*retlen = 0;
 		return -EINVAL;
 	}
 
-	/* Grab the lock and see if the device is available */
-	onenand_get_device(mtd, FL_READING);
-
 	stats = mtd->ecc_stats;
 
 	/* Read-while-load method */
@@ -804,9 +792,6 @@ static int onenand_read(struct mtd_info *mtd, loff_t from, size_t len,
 		onenand_update_bufferram(mtd, from, !ret);
 	}
 
-	/* Deselect and wake up anyone waiting on the device */
-	onenand_release_device(mtd);
-
 	/*
 	 * Return success, if no ECC failures, else -EBADMSG
 	 * fs driver will take care of that, because
@@ -824,6 +809,28 @@ static int onenand_read(struct mtd_info *mtd, loff_t from, size_t len,
 }
 
 /**
+ * onenand_read - [MTD Interface] Read data from flash
+ * @param mtd		MTD device structure
+ * @param from		offset to read from
+ * @param len		number of bytes to read
+ * @param retlen	pointer to variable to store the number of read bytes
+ * @param buf		the databuffer to put data
+ *
+ * Read with ecc
+*/
+static int onenand_read(struct mtd_info *mtd, loff_t from, size_t len,
+			size_t *retlen, u_char *buf)
+{
+	int ret;
+
+	onenand_get_device(mtd, FL_READING);
+	ret = onenand_read_nolock(mtd, from, len, retlen, buf);
+	onenand_release_device(mtd);
+
+	return ret;
+}
+
+/**
  * onenand_transfer_auto_oob - [Internal] oob auto-placement transfer
  * @param mtd		MTD device structure
  * @param buf		destination address
@@ -863,7 +870,7 @@ static int onenand_transfer_auto_oob(struct mtd_info *mtd, uint8_t *buf, int col
 }
 
 /**
- * onenand_do_read_oob - [MTD Interface] OneNAND read out-of-band
+ * onenand_read_oob_nolock - [MTD Interface] OneNAND read out-of-band
  * @param mtd		MTD device structure
  * @param from		offset to read from
  * @param len		number of bytes to read
@@ -873,14 +880,15 @@ static int onenand_transfer_auto_oob(struct mtd_info *mtd, uint8_t *buf, int col
  *
  * OneNAND read out-of-band data from the spare area
  */
-static int onenand_do_read_oob(struct mtd_info *mtd, loff_t from, size_t len,
+static int onenand_read_oob_nolock(struct mtd_info *mtd, loff_t from, size_t len,
 			size_t *retlen, u_char *buf, mtd_oob_mode_t mode)
 {
 	struct onenand_chip *this = mtd->priv;
 	int read = 0, thislen, column, oobsize;
 	int ret = 0;
 
-	DEBUG(MTD_DEBUG_LEVEL3, "onenand_read_oob: from = 0x%08x, len = %i\n", (unsigned int) from, (int) len);
+	DEBUG(MTD_DEBUG_LEVEL3, "onenand_read_oob_nolock: from = 0x%08x, len = %i\n",
+	      (unsigned int) from, (int) len);
 
 	/* Initialize return length value */
 	*retlen = 0;
@@ -893,7 +901,7 @@ static int onenand_do_read_oob(struct mtd_info *mtd, loff_t from, size_t len,
 	column = from & (mtd->oobsize - 1);
 
 	if (unlikely(column >= oobsize)) {
-		printk(KERN_ERR "onenand_read_oob: Attempted to start read outside oob\n");
+		printk(KERN_ERR "onenand_read_oob_nolock: Attempted to start read outside oob\n");
 		return -EINVAL;
 	}
 
@@ -901,13 +909,10 @@ static int onenand_do_read_oob(struct mtd_info *mtd, loff_t from, size_t len,
 	if (unlikely(from >= mtd->size ||
 		     column + len > ((mtd->size >> this->page_shift) -
 				     (from >> this->page_shift)) * oobsize)) {
-		printk(KERN_ERR "onenand_read_oob: Attempted to read beyond end of device\n");
+		printk(KERN_ERR "onenand_read_oob_nolock: Attempted to read beyond end of device\n");
 		return -EINVAL;
 	}
 
-	/* Grab the lock and see if the device is available */
-	onenand_get_device(mtd, FL_READING);
-
 	while (read < len) {
 		cond_resched();
 
@@ -927,7 +932,7 @@ static int onenand_do_read_oob(struct mtd_info *mtd, loff_t from, size_t len,
 			this->read_bufferram(mtd, ONENAND_SPARERAM, buf, column, thislen);
 
 		if (ret) {
-			printk(KERN_ERR "onenand_read_oob: read failed = 0x%x\n", ret);
+			printk(KERN_ERR "onenand_read_oob_nolock: read failed = 0x%x\n", ret);
 			break;
 		}
 
@@ -946,9 +951,6 @@ static int onenand_do_read_oob(struct mtd_info *mtd, loff_t from, size_t len,
 		}
 	}
 
-	/* Deselect and wake up anyone waiting on the device */
-	onenand_release_device(mtd);
-
 	*retlen = read;
 	return ret;
 }
@@ -962,6 +964,8 @@ static int onenand_do_read_oob(struct mtd_info *mtd, loff_t from, size_t len,
 static int onenand_read_oob(struct mtd_info *mtd, loff_t from,
 			    struct mtd_oob_ops *ops)
 {
+	int ret;
+
 	switch (ops->mode) {
 	case MTD_OOB_PLACE:
 	case MTD_OOB_AUTO:
@@ -971,8 +975,12 @@ static int onenand_read_oob(struct mtd_info *mtd, loff_t from,
 	default:
 		return -EINVAL;
 	}
-	return onenand_do_read_oob(mtd, from + ops->ooboffs, ops->ooblen,
-				   &ops->oobretlen, ops->oobbuf, ops->mode);
+
+	onenand_get_device(mtd, FL_READING);
+	ret = onenand_read_oob_nolock(mtd, from + ops->ooboffs, ops->ooblen,
+				      &ops->oobretlen, ops->oobbuf, ops->mode);
+	onenand_release_device(mtd);
+	return ret;
 }
 
 /**
@@ -1169,46 +1177,34 @@ static int onenand_verify(struct mtd_info *mtd, const u_char *buf, loff_t addr,
 
 #define NOTALIGNED(x)	((x & (this->subpagesize - 1)) != 0)
 
-/**
- * onenand_write - [MTD Interface] write buffer to FLASH
- * @param mtd		MTD device structure
- * @param to		offset to write to
- * @param len		number of bytes to write
- * @param retlen	pointer to variable to store the number of written bytes
- * @param buf		the data to write
- *
- * Write with ECC
- */
-static int onenand_write(struct mtd_info *mtd, loff_t to, size_t len,
-	size_t *retlen, const u_char *buf)
+static int onenand_write_nolock(struct mtd_info *mtd, loff_t to, size_t len,
+				size_t *retlen, const u_char *buf)
 {
 	struct onenand_chip *this = mtd->priv;
 	int written = 0;
 	int ret = 0;
 	int column, subpage;
 
-	DEBUG(MTD_DEBUG_LEVEL3, "onenand_write: to = 0x%08x, len = %i\n", (unsigned int) to, (int) len);
+	DEBUG(MTD_DEBUG_LEVEL3, "onenand_write_nolock: to = 0x%08x, len = %i\n",
+	      (unsigned int) to, (int) len);
 
 	/* Initialize retlen, in case of early exit */
 	*retlen = 0;
 
 	/* Do not allow writes past end of device */
 	if (unlikely((to + len) > mtd->size)) {
-		printk(KERN_ERR "onenand_write: Attempt write to past end of device\n");
+		printk(KERN_ERR "onenand_write_nolock: Attempt write to past end of device\n");
 		return -EINVAL;
 	}
 
 	/* Reject writes, which are not page aligned */
         if (unlikely(NOTALIGNED(to)) || unlikely(NOTALIGNED(len))) {
-                printk(KERN_ERR "onenand_write: Attempt to write not page aligned data\n");
+                printk(KERN_ERR "onenand_write_nolock: Attempt to write not page aligned data\n");
                 return -EINVAL;
         }
 
 	column = to & (mtd->writesize - 1);
 
-	/* Grab the lock and see if the device is available */
-	onenand_get_device(mtd, FL_WRITING);
-
 	/* Loop until all data write */
 	while (written < len) {
 		int thislen = min_t(int, mtd->writesize - column, len - written);
@@ -1237,14 +1233,14 @@ static int onenand_write(struct mtd_info *mtd, loff_t to, size_t len,
 		onenand_update_bufferram(mtd, to, !ret && !subpage);
 
 		if (ret) {
-			printk(KERN_ERR "onenand_write: write filaed %d\n", ret);
+			printk(KERN_ERR "onenand_write_nolock: write filaed %d\n", ret);
 			break;
 		}
 
 		/* Only check verify write turn on */
 		ret = onenand_verify(mtd, (u_char *) wbuf, to, thislen);
 		if (ret) {
-			printk(KERN_ERR "onenand_write: verify failed %d\n", ret);
+			printk(KERN_ERR "onenand_write_nolock: verify failed %d\n", ret);
 			break;
 		}
 
@@ -1258,10 +1254,28 @@ static int onenand_write(struct mtd_info *mtd, loff_t to, size_t len,
 		buf += thislen;
 	}
 
-	/* Deselect and wake up anyone waiting on the device */
-	onenand_release_device(mtd);
-
 	*retlen = written;
+	return ret;
+}
+
+/**
+ * onenand_write - [MTD Interface] write buffer to FLASH
+ * @param mtd		MTD device structure
+ * @param to		offset to write to
+ * @param len		number of bytes to write
+ * @param retlen	pointer to variable to store the number of written bytes
+ * @param buf		the data to write
+ *
+ * Write with ECC
+ */
+static int onenand_write(struct mtd_info *mtd, loff_t to, size_t len,
+			 size_t *retlen, const u_char *buf)
+{
+	int ret;
+
+	onenand_get_device(mtd, FL_WRITING);
+	ret = onenand_write_nolock(mtd, to, len, retlen, buf);
+	onenand_release_device(mtd);
 
 	return ret;
 }
@@ -1305,7 +1319,7 @@ static int onenand_fill_auto_oob(struct mtd_info *mtd, u_char *oob_buf,
 }
 
 /**
- * onenand_do_write_oob - [Internal] OneNAND write out-of-band
+ * onenand_write_oob_nolock - [Internal] OneNAND write out-of-band
  * @param mtd		MTD device structure
  * @param to		offset to write to
  * @param len		number of bytes to write
@@ -1315,7 +1329,7 @@ static int onenand_fill_auto_oob(struct mtd_info *mtd, u_char *oob_buf,
  *
  * OneNAND write out-of-band
  */
-static int onenand_do_write_oob(struct mtd_info *mtd, loff_t to, size_t len,
+static int onenand_write_oob_nolock(struct mtd_info *mtd, loff_t to, size_t len,
 				size_t *retlen, const u_char *buf, mtd_oob_mode_t mode)
 {
 	struct onenand_chip *this = mtd->priv;
@@ -1323,7 +1337,8 @@ static int onenand_do_write_oob(struct mtd_info *mtd, loff_t to, size_t len,
 	int written = 0;
 	u_char *oobbuf;
 
-	DEBUG(MTD_DEBUG_LEVEL3, "onenand_write_oob: to = 0x%08x, len = %i\n", (unsigned int) to, (int) len);
+	DEBUG(MTD_DEBUG_LEVEL3, "onenand_write_oob_nolock: to = 0x%08x, len = %i\n",
+	      (unsigned int) to, (int) len);
 
 	/* Initialize retlen, in case of early exit */
 	*retlen = 0;
@@ -1336,13 +1351,13 @@ static int onenand_do_write_oob(struct mtd_info *mtd, loff_t to, size_t len,
 	column = to & (mtd->oobsize - 1);
 
 	if (unlikely(column >= oobsize)) {
-		printk(KERN_ERR "onenand_write_oob: Attempted to start write outside oob\n");
+		printk(KERN_ERR "onenand_write_oob_nolock: Attempted to start write outside oob\n");
 		return -EINVAL;
 	}
 
 	/* For compatibility with NAND: Do not allow write past end of page */
 	if (unlikely(column + len > oobsize)) {
-		printk(KERN_ERR "onenand_write_oob: "
+		printk(KERN_ERR "onenand_write_oob_nolock: "
 		      "Attempt to write past end of page\n");
 		return -EINVAL;
 	}
@@ -1351,13 +1366,10 @@ static int onenand_do_write_oob(struct mtd_info *mtd, loff_t to, size_t len,
 	if (unlikely(to >= mtd->size ||
 		     column + len > ((mtd->size >> this->page_shift) -
 				     (to >> this->page_shift)) * oobsize)) {
-		printk(KERN_ERR "onenand_write_oob: Attempted to write past end of device\n");
+		printk(KERN_ERR "onenand_write_oob_nolock: Attempted to write past end of device\n");
 		return -EINVAL;
 	}
 
-	/* Grab the lock and see if the device is available */
-	onenand_get_device(mtd, FL_WRITING);
-
 	oobbuf = this->oob_buf;
 
 	/* Loop until all data write */
@@ -1383,13 +1395,13 @@ static int onenand_do_write_oob(struct mtd_info *mtd, loff_t to, size_t len,
 
 		ret = this->wait(mtd, FL_WRITING);
 		if (ret) {
-			printk(KERN_ERR "onenand_write_oob: write failed %d\n", ret);
+			printk(KERN_ERR "onenand_write_oob_nolock: write failed %d\n", ret);
 			break;
 		}
 
 		ret = onenand_verify_oob(mtd, oobbuf, to);
 		if (ret) {
-			printk(KERN_ERR "onenand_write_oob: verify failed %d\n", ret);
+			printk(KERN_ERR "onenand_write_oob_nolock: verify failed %d\n", ret);
 			break;
 		}
 
@@ -1402,11 +1414,7 @@ static int onenand_do_write_oob(struct mtd_info *mtd, loff_t to, size_t len,
 		column = 0;
 	}
 
-	/* Deselect and wake up anyone waiting on the device */
-	onenand_release_device(mtd);
-
 	*retlen = written;
-
 	return ret;
 }
 
@@ -1419,6 +1427,8 @@ static int onenand_do_write_oob(struct mtd_info *mtd, loff_t to, size_t len,
 static int onenand_write_oob(struct mtd_info *mtd, loff_t to,
 			     struct mtd_oob_ops *ops)
 {
+	int ret;
+
 	switch (ops->mode) {
 	case MTD_OOB_PLACE:
 	case MTD_OOB_AUTO:
@@ -1428,21 +1438,15 @@ static int onenand_write_oob(struct mtd_info *mtd, loff_t to,
 	default:
 		return -EINVAL;
 	}
-	return onenand_do_write_oob(mtd, to + ops->ooboffs, ops->ooblen,
+	onenand_get_device(mtd, FL_WRITING);
+	ret =  onenand_write_oob_nolock(mtd, to + ops->ooboffs, ops->ooblen,
 				    &ops->oobretlen, ops->oobbuf, ops->mode);
+	onenand_release_device(mtd);
+	return ret;
 }
 
-/**
- * onenand_block_checkbad - [GENERIC] Check if a block is marked bad
- * @param mtd		MTD device structure
- * @param ofs		offset from device start
- * @param getchip	0, if the chip is already selected
- * @param allowbbt	1, if its allowed to access the bbt area
- *
- * Check, if the block is bad. Either by reading the bad block table or
- * calling of the scan function.
- */
-static int onenand_block_checkbad(struct mtd_info *mtd, loff_t ofs, int getchip, int allowbbt)
+static int onenand_block_isbad_nolock(struct mtd_info *mtd, loff_t ofs,
+				      int allowbbt)
 {
 	struct onenand_chip *this = mtd->priv;
 	struct bbm_info *bbm = this->bbm;
@@ -1503,7 +1507,7 @@ static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
 		cond_resched();
 
 		/* Check if we have a bad block, we do not erase bad blocks */
-		if (onenand_block_checkbad(mtd, addr, 0, 0)) {
+		if (onenand_block_isbad_nolock(mtd, addr, 0)) {
 			printk (KERN_WARNING "onenand_erase: attempt to erase a bad block at addr 0x%08x\n", (unsigned int) addr);
 			instr->state = MTD_ERASE_FAILED;
 			goto erase_exit;
@@ -1567,11 +1571,16 @@ static void onenand_sync(struct mtd_info *mtd)
  */
 static int onenand_block_isbad(struct mtd_info *mtd, loff_t ofs)
 {
+	int ret;
+
 	/* Check for invalid offset */
 	if (ofs > mtd->size)
 		return -EINVAL;
 
-	return onenand_block_checkbad(mtd, ofs, 1, 0);
+	onenand_get_device(mtd, FL_READING);
+	ret = onenand_block_isbad_nolock(mtd, ofs, 0);
+	onenand_release_device(mtd);
+	return ret;
 }
 
 /**
@@ -1588,7 +1597,7 @@ static int onenand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
 	struct bbm_info *bbm = this->bbm;
 	u_char buf[2] = {0, 0};
 	size_t retlen;
-	int block;
+	int block, ret;
 
 	/* Get block number */
 	block = ((int) ofs) >> bbm->bbt_erase_shift;
@@ -1597,7 +1606,8 @@ static int onenand_default_block_markbad(struct mtd_info *mtd, loff_t ofs)
 
         /* We write two bytes, so we dont have to mess with 16 bit access */
         ofs += mtd->oobsize + (bbm->badblockpos & ~0x01);
-        return onenand_do_write_oob(mtd, ofs , 2, &retlen, buf, MTD_OOB_PLACE);
+        ret = onenand_write_oob_nolock(mtd, ofs , 2, &retlen, buf, MTD_OOB_PLACE);
+	return ret;
 }
 
 /**
@@ -1620,7 +1630,10 @@ static int onenand_block_markbad(struct mtd_info *mtd, loff_t ofs)
 		return ret;
 	}
 
-	return this->block_markbad(mtd, ofs);
+	onenand_get_device(mtd, FL_WRITING);
+	ret = this->block_markbad(mtd, ofs);
+	onenand_release_device(mtd);
+	return ret;
 }
 
 /**
@@ -1825,7 +1838,7 @@ static int do_otp_read(struct mtd_info *mtd, loff_t from, size_t len,
 	this->command(mtd, ONENAND_CMD_OTP_ACCESS, 0, 0);
 	this->wait(mtd, FL_OTPING);
 
-	ret = mtd->read(mtd, from, len, retlen, buf);
+	ret = onenand_read_nolock(mtd, from, len, retlen, buf);
 
 	/* Exit OTP access mode */
 	this->command(mtd, ONENAND_CMD_RESET, 0, 0);
@@ -1837,14 +1850,14 @@ static int do_otp_read(struct mtd_info *mtd, loff_t from, size_t len,
 /**
  * do_otp_write - [DEFAULT] Write OTP block area
  * @param mtd		MTD device structure
- * @param from		The offset to write
+ * @param to		The offset to write
  * @param len		number of bytes to write
  * @param retlen	pointer to variable to store the number of write bytes
  * @param buf		the databuffer to put/get data
  *
  * Write OTP block area.
  */
-static int do_otp_write(struct mtd_info *mtd, loff_t from, size_t len,
+static int do_otp_write(struct mtd_info *mtd, loff_t to, size_t len,
 		size_t *retlen, u_char *buf)
 {
 	struct onenand_chip *this = mtd->priv;
@@ -1863,7 +1876,7 @@ static int do_otp_write(struct mtd_info *mtd, loff_t from, size_t len,
 	this->command(mtd, ONENAND_CMD_OTP_ACCESS, 0, 0);
 	this->wait(mtd, FL_OTPING);
 
-	ret = mtd->write(mtd, from, len, retlen, pbuf);
+	ret = onenand_write_nolock(mtd, to, len, retlen, pbuf);
 
 	/* Exit OTP access mode */
 	this->command(mtd, ONENAND_CMD_RESET, 0, 0);
@@ -1892,7 +1905,8 @@ static int do_otp_lock(struct mtd_info *mtd, loff_t from, size_t len,
 	this->command(mtd, ONENAND_CMD_OTP_ACCESS, 0, 0);
 	this->wait(mtd, FL_OTPING);
 
-	ret = onenand_do_write_oob(mtd, from, len, retlen, buf, MTD_OOB_PLACE);
+	ret = onenand_write_oob_nolock(mtd, from, len, retlen, buf,
+				       MTD_OOB_PLACE);
 
 	/* Exit OTP access mode */
 	this->command(mtd, ONENAND_CMD_RESET, 0, 0);
@@ -1939,13 +1953,16 @@ static int onenand_otp_walk(struct mtd_info *mtd, loff_t from, size_t len,
 	if (((mtd->writesize * otp_pages) - (from + len)) < 0)
 		return 0;
 
+	onenand_get_device(mtd, FL_OTPING);
 	while (len > 0 && otp_pages > 0) {
 		if (!action) {	/* OTP Info functions */
 			struct otp_info *otpinfo;
 
 			len -= sizeof(struct otp_info);
-			if (len <= 0)
-				return -ENOSPC;
+			if (len <= 0) {
+				ret = -ENOSPC;
+				break;
+			}
 
 			otpinfo = (struct otp_info *) buf;
 			otpinfo->start = from;
@@ -1966,10 +1983,11 @@ static int onenand_otp_walk(struct mtd_info *mtd, loff_t from, size_t len,
 			*retlen += size;
 
 			if (ret < 0)
-				return ret;
+				break;
 		}
 		otp_pages--;
 	}
+	onenand_release_device(mtd);
 
 	return 0;
 }
-- 
1.5.2.2


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