[PATCH v5 0/3] MTD: OneNAND: multiblock erase support

[PATCH v5 0/3] MTD: OneNAND: multiblock erase support

[Mika Korhonen]

(v5) Adjust patches to apply on current tree. Use __func__ in printk's.
     Fix mistakes found by K. Park.

(v4) This patch series is a reworked version of

http://lists.infradead.org/pipermail/linux-mtd/2009-September/027138.html
http://lists.infradead.org/pipermail/linux-mtd/2009-September/027139.html
http://lists.infradead.org/pipermail/linux-mtd/2009-September/027140.html

based on comments by Adrian Hunter and Kyungmin Park. This version should
address the issues pointed out by them.

The original patch is:
http://lists.infradead.org/pipermail/linux-mtd/2009-June/026130.html

I split the original patch in two for readability: the first part extracts
the execution of erase command to a separate function for easier integration
of different erase method. The second part implements the multiblock erase
function in case multiple blocks are requested to be erased and the chip is
not Flex.

This is useful for flashing applications that need to do their work as
fast as possible. For full 64 eraseblock case the erase speed is up to 30x
faster. (Samsung: 64 MB/s vs 2.1 GB/s, I got 1.4 GB/s on Linux kernel)


Mika Korhonen (3):
  MTD: OneNAND: move erase method to a separate function
  MTD: OneNAND: multiblock erase support
  MTD: OneNAND: fix double printing of function name

 drivers/mtd/onenand/omap2.c        |   22 +++-
 drivers/mtd/onenand/onenand_base.c |  289 +++++++++++++++++++++++++++++------
 include/linux/mtd/flashchip.h      |    4 +-
 include/linux/mtd/onenand_regs.h   |    2 +
 4 files changed, 262 insertions(+), 55 deletions(-)

[PATCH v5 1/3] MTD: OneNAND: move erase method to a separate function

[Mika Korhonen]

Separate the actual execution of erase to a new function:
onenand_block_by_block_erase(). This is done in preparation for
the multiblock erase support.

Signed-off-by: Mika Korhonen <ext-mika.2.korhonen@nokia.com>
---
 drivers/mtd/onenand/onenand_base.c |  132 +++++++++++++++++++++---------------
 1 files changed, 76 insertions(+), 56 deletions(-)

diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 6e250f3..51f4782 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -2169,69 +2169,33 @@ static int onenand_block_isbad_nolock(struct mtd_info *mtd, loff_t ofs, int allo
 }
 
 /**
- * onenand_erase - [MTD Interface] erase block(s)
+ * onenand_block_by_block_erase - [Internal] erase block(s) using regular erase
  * @param mtd		MTD device structure
  * @param instr		erase instruction
+ * @param region	erase region
+ * @param block_size	erase block size
  *
- * Erase one ore more blocks
+ * Erase one or more blocks one block at a time
  */
-static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
+static int onenand_block_by_block_erase(struct mtd_info *mtd,
+					struct erase_info *instr,
+					struct mtd_erase_region_info *region,
+					unsigned int block_size)
 {
 	struct onenand_chip *this = mtd->priv;
-	unsigned int block_size;
 	loff_t addr = instr->addr;
-	loff_t len = instr->len;
-	int ret = 0, i;
-	struct mtd_erase_region_info *region = NULL;
+	int len = instr->len;
 	loff_t region_end = 0;
+	int ret = 0;
 
-	DEBUG(MTD_DEBUG_LEVEL3, "onenand_erase: start = 0x%012llx, len = %llu\n", (unsigned long long) instr->addr, (unsigned long long) instr->len);
-
-	/* Do not allow erase past end of device */
-	if (unlikely((len + addr) > mtd->size)) {
-		printk(KERN_ERR "%s: Erase past end of device\n", __func__);
-		return -EINVAL;
-	}
-
-	if (FLEXONENAND(this)) {
-		/* Find the eraseregion of this address */
-		i = flexonenand_region(mtd, addr);
-		region = &mtd->eraseregions[i];
-
-		block_size = region->erasesize;
+	if (region) {
+		/* region is set for Flex-OneNAND */
 		region_end = region->offset + region->erasesize * region->numblocks;
-
-		/* Start address within region must align on block boundary.
-		 * Erase region's start offset is always block start address.
-		 */
-		if (unlikely((addr - region->offset) & (block_size - 1))) {
-			printk(KERN_ERR "%s: Unaligned address\n", __func__);
-			return -EINVAL;
-		}
-	} else {
-		block_size = 1 << this->erase_shift;
-
-		/* Start address must align on block boundary */
-		if (unlikely(addr & (block_size - 1))) {
-			printk(KERN_ERR "%s: Unaligned address\n", __func__);
-			return -EINVAL;
-		}
-	}
-
-	/* Length must align on block boundary */
-	if (unlikely(len & (block_size - 1))) {
-		printk(KERN_ERR "%s: Length not block aligned\n", __func__);
-		return -EINVAL;
 	}
 
-	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
-
-	/* Grab the lock and see if the device is available */
-	onenand_get_device(mtd, FL_ERASING);
-
-	/* Loop through the blocks */
 	instr->state = MTD_ERASING;
 
+	/* Loop through the blocks */
 	while (len) {
 		cond_resched();
 
@@ -2241,7 +2205,7 @@ static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
 					"at addr 0x%012llx\n",
 					__func__, (unsigned long long) addr);
 			instr->state = MTD_ERASE_FAILED;
-			goto erase_exit;
+			return -EIO;
 		}
 
 		this->command(mtd, ONENAND_CMD_ERASE, addr, block_size);
@@ -2255,7 +2219,7 @@ static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
 				__func__, onenand_block(this, addr));
 			instr->state = MTD_ERASE_FAILED;
 			instr->fail_addr = addr;
-			goto erase_exit;
+			return -EIO;
 		}
 
 		len -= block_size;
@@ -2273,24 +2237,80 @@ static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
 				/* FIXME: This should be handled at MTD partitioning level. */
 				printk(KERN_ERR "%s: Unaligned address\n",
 					__func__);
-				goto erase_exit;
+				return -EIO;
 			}
 		}
+	}
+	return 0;
+}
+
+/**
+ * onenand_erase - [MTD Interface] erase block(s)
+ * @param mtd		MTD device structure
+ * @param instr		erase instruction
+ *
+ * Erase one or more blocks
+ */
+static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
+{
+	struct onenand_chip *this = mtd->priv;
+	unsigned int block_size;
+	loff_t addr = instr->addr;
+	loff_t len = instr->len;
+	int ret = 0;
+	struct mtd_erase_region_info *region = NULL;
+	loff_t region_offset = 0;
+
+	DEBUG(MTD_DEBUG_LEVEL3, "%s: start=0x%012llx, len=%llu\n", __func__,
+	      (unsigned long long) instr->addr, (unsigned long long) instr->len);
+
+	/* Do not allow erase past end of device */
+	if (unlikely((len + addr) > mtd->size)) {
+		printk(KERN_ERR "%s: Erase past end of device\n", __func__);
+		return -EINVAL;
+	}
+
+	if (FLEXONENAND(this)) {
+		/* Find the eraseregion of this address */
+		int i = flexonenand_region(mtd, addr);
+
+		region = &mtd->eraseregions[i];
+		block_size = region->erasesize;
+
+		/* Start address within region must align on block boundary.
+		 * Erase region's start offset is always block start address.
+		 */
+		region_offset = region->offset;
+	} else
+		block_size = 1 << this->erase_shift;
+
+	/* Start address must align on block boundary */
+	if (unlikely((addr - region_offset) & (block_size - 1))) {
+		printk(KERN_ERR "%s: Unaligned address\n", __func__);
+		return -EINVAL;
+	}
 
+	/* Length must align on block boundary */
+	if (unlikely(len & (block_size - 1))) {
+		printk(KERN_ERR "%s: Length not block aligned\n", __func__);
+		return -EINVAL;
 	}
 
-	instr->state = MTD_ERASE_DONE;
+	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
 
-erase_exit:
+	/* Grab the lock and see if the device is available */
+	onenand_get_device(mtd, FL_ERASING);
 
-	ret = instr->state == MTD_ERASE_DONE ? 0 : -EIO;
+	ret = onenand_block_by_block_erase(mtd, instr, region, block_size);
 
 	/* Deselect and wake up anyone waiting on the device */
 	onenand_release_device(mtd);
 
 	/* Do call back function */
-	if (!ret)
+	if (!ret) {
+		instr->state = MTD_ERASE_DONE;
 		mtd_erase_callback(instr);
+	}
 
 	return ret;
 }
-- 
1.6.0.4

[PATCH v5 2/3] MTD: OneNAND: multiblock erase support

[Mika Korhonen]

Add support for multiblock erase command. OneNANDs (excluding Flex-OneNAND)
are capable of simultaneous erase of up to 64 eraseblocks which is much faster.

This changes the erase requests for regions covering multiple eraseblocks
to be performed using multiblock erase.

Signed-off-by: Mika Korhonen <ext-mika.2.korhonen@nokia.com>
---
 drivers/mtd/onenand/omap2.c        |   22 ++++-
 drivers/mtd/onenand/onenand_base.c |  173 +++++++++++++++++++++++++++++++++++-
 include/linux/mtd/flashchip.h      |    4 +-
 include/linux/mtd/onenand_regs.h   |    2 +
 4 files changed, 194 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c
index 0108ed4..2dafd09 100644
--- a/drivers/mtd/onenand/omap2.c
+++ b/drivers/mtd/onenand/omap2.c
@@ -112,10 +112,24 @@ static int omap2_onenand_wait(struct mtd_info *mtd, int state)
 	unsigned long timeout;
 	u32 syscfg;
 
-	if (state == FL_RESETING) {
-		int i;
+	if (state == FL_RESETING || state == FL_PREPARING_ERASE ||
+	    state == FL_VERIFYING_ERASE) {
+		int i = 21;
+		unsigned int intr_flags = ONENAND_INT_MASTER;
+
+		switch (state) {
+		case FL_RESETING:
+			intr_flags |= ONENAND_INT_RESET;
+			break;
+		case FL_PREPARING_ERASE:
+			intr_flags |= ONENAND_INT_ERASE;
+			break;
+		case FL_VERIFYING_ERASE:
+			i = 101;
+			break;
+		}
 
-		for (i = 0; i < 20; i++) {
+		while (--i) {
 			udelay(1);
 			intr = read_reg(c, ONENAND_REG_INTERRUPT);
 			if (intr & ONENAND_INT_MASTER)
@@ -126,7 +140,7 @@ static int omap2_onenand_wait(struct mtd_info *mtd, int state)
 			wait_err("controller error", state, ctrl, intr);
 			return -EIO;
 		}
-		if (!(intr & ONENAND_INT_RESET)) {
+		if ((intr & intr_flags) != intr_flags) {
 			wait_err("timeout", state, ctrl, intr);
 			return -EIO;
 		}
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 51f4782..7355f62 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -32,6 +32,13 @@
 
 #include <asm/io.h>
 
+/*
+ * Multiblock erase if number of blocks to erase is 2 or more.
+ * Maximum number of blocks for simultaneous erase is 64.
+ */
+#define MB_ERASE_MIN_BLK_COUNT 2
+#define MB_ERASE_MAX_BLK_COUNT 64
+
 /* Default Flex-OneNAND boundary and lock respectively */
 static int flex_bdry[MAX_DIES * 2] = { -1, 0, -1, 0 };
 
@@ -339,6 +346,8 @@ static int onenand_command(struct mtd_info *mtd, int cmd, loff_t addr, size_t le
 		break;
 
 	case ONENAND_CMD_ERASE:
+	case ONENAND_CMD_MULTIBLOCK_ERASE:
+	case ONENAND_CMD_ERASE_VERIFY:
 	case ONENAND_CMD_BUFFERRAM:
 	case ONENAND_CMD_OTP_ACCESS:
 		block = onenand_block(this, addr);
@@ -483,7 +492,7 @@ static int onenand_wait(struct mtd_info *mtd, int state)
 		if (interrupt & flags)
 			break;
 
-		if (state != FL_READING)
+		if (state != FL_READING && state != FL_PREPARING_ERASE)
 			cond_resched();
 	}
 	/* To get correct interrupt status in timeout case */
@@ -516,6 +525,18 @@ static int onenand_wait(struct mtd_info *mtd, int state)
 		return -EIO;
 	}
 
+	if (state == FL_PREPARING_ERASE && !(interrupt & ONENAND_INT_ERASE)) {
+		printk(KERN_ERR "%s: mb erase timeout! ctrl=0x%04x intr=0x%04x\n",
+		       __func__, ctrl, interrupt);
+		return -EIO;
+	}
+
+	if (!(interrupt & ONENAND_INT_MASTER)) {
+		printk(KERN_ERR "%s: timeout! ctrl=0x%04x intr=0x%04x\n",
+		       __func__, ctrl, interrupt);
+		return -EIO;
+	}
+
 	/* If there's controller error, it's a real error */
 	if (ctrl & ONENAND_CTRL_ERROR) {
 		printk(KERN_ERR "%s: controller error = 0x%04x\n",
@@ -2168,6 +2189,148 @@ static int onenand_block_isbad_nolock(struct mtd_info *mtd, loff_t ofs, int allo
 	return bbm->isbad_bbt(mtd, ofs, allowbbt);
 }
 
+
+static int onenand_multiblock_erase_verify(struct mtd_info *mtd,
+					   struct erase_info *instr)
+{
+	struct onenand_chip *this = mtd->priv;
+	loff_t addr = instr->addr;
+	int len = instr->len;
+	unsigned int block_size = (1 << this->erase_shift);
+	int ret = 0;
+
+	while (len) {
+		this->command(mtd, ONENAND_CMD_ERASE_VERIFY, addr, block_size);
+		ret = this->wait(mtd, FL_VERIFYING_ERASE);
+		if (ret) {
+			printk(KERN_ERR "%s: Failed verify, block %d\n",
+			       __func__, onenand_block(this, addr));
+			instr->state = MTD_ERASE_FAILED;
+			instr->fail_addr = addr;
+			return -1;
+		}
+		len -= block_size;
+		addr += block_size;
+	}
+	return 0;
+}
+
+/**
+ * onenand_multiblock_erase - [Internal] erase block(s) using multiblock erase
+ * @param mtd		MTD device structure
+ * @param instr		erase instruction
+ * @param region	erase region
+ *
+ * Erase one or more blocks up to 64 block at a time
+ */
+static int onenand_multiblock_erase(struct mtd_info *mtd,
+				    struct erase_info *instr,
+				    unsigned int block_size)
+{
+	struct onenand_chip *this = mtd->priv;
+	loff_t addr = instr->addr;
+	int len = instr->len;
+	int eb_count = 0;
+	int ret = 0;
+	int bdry_block = 0;
+
+	instr->state = MTD_ERASING;
+
+	if (ONENAND_IS_DDP(this)) {
+		loff_t bdry_addr = this->chipsize >> 1;
+		if (addr < bdry_addr && (addr + len) > bdry_addr)
+			bdry_block = bdry_addr >> this->erase_shift;
+	}
+
+	/* Pre-check bbs */
+	while (len) {
+		/* Check if we have a bad block, we do not erase bad blocks */
+		if (onenand_block_isbad_nolock(mtd, addr, 0)) {
+			printk(KERN_WARNING "%s: attempt to erase a bad block "
+			       "at addr 0x%012llx\n",
+			       __func__, (unsigned long long) addr);
+			instr->state = MTD_ERASE_FAILED;
+			return -EIO;
+		}
+		len -= block_size;
+		addr += block_size;
+	}
+
+	len = instr->len;
+	addr = instr->addr;
+
+	/* loop over 64 eb batches */
+	while (len) {
+		struct erase_info verify_instr = *instr;
+		int max_eb_count = MB_ERASE_MAX_BLK_COUNT;
+
+		verify_instr.addr = addr;
+		verify_instr.len = 0;
+
+		/* do not cross chip boundary */
+		if (bdry_block) {
+			int this_block = (addr >> this->erase_shift);
+
+			if (this_block < bdry_block) {
+				max_eb_count = min(max_eb_count,
+						   (bdry_block - this_block));
+			}
+		}
+
+		eb_count = 0;
+
+		while (len > block_size && eb_count < (max_eb_count - 1)) {
+			this->command(mtd, ONENAND_CMD_MULTIBLOCK_ERASE,
+				      addr, block_size);
+			onenand_invalidate_bufferram(mtd, addr, block_size);
+
+			ret = this->wait(mtd, FL_PREPARING_ERASE);
+			if (ret) {
+				printk(KERN_ERR "%s: Failed multiblock erase, "
+				       "block %d\n", __func__,
+				       onenand_block(this, addr));
+				instr->state = MTD_ERASE_FAILED;
+				instr->fail_addr = addr;
+				return -EIO;
+			}
+
+			len -= block_size;
+			addr += block_size;
+			eb_count++;
+		}
+
+		/* last block of 64-eb series */
+		cond_resched();
+		this->command(mtd, ONENAND_CMD_ERASE, addr, block_size);
+		onenand_invalidate_bufferram(mtd, addr, block_size);
+
+		ret = this->wait(mtd, FL_ERASING);
+		/* Check if it is write protected */
+		if (ret) {
+			printk(KERN_ERR "%s: Failed erase, block %d\n",
+			       __func__, onenand_block(this, addr));
+			instr->state = MTD_ERASE_FAILED;
+			instr->fail_addr = addr;
+			return -EIO;
+		}
+
+		len -= block_size;
+		addr += block_size;
+		eb_count++;
+
+		/* verify */
+		verify_instr.len = eb_count * block_size;
+		if (onenand_multiblock_erase_verify(mtd, &verify_instr)) {
+			instr->state = verify_instr.state;
+			instr->fail_addr = verify_instr.fail_addr;
+			return -EIO;
+		}
+
+	}
+	return 0;
+}
+
+
 /**
  * onenand_block_by_block_erase - [Internal] erase block(s) using regular erase
  * @param mtd		MTD device structure
@@ -2301,7 +2464,13 @@ static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
 	/* Grab the lock and see if the device is available */
 	onenand_get_device(mtd, FL_ERASING);
 
-	ret = onenand_block_by_block_erase(mtd, instr, region, block_size);
+	if (region || instr->len < MB_ERASE_MIN_BLK_COUNT * block_size) {
+		/* region is set for Flex-OneNAND (no mb erase) */
+		ret = onenand_block_by_block_erase(mtd, instr,
+						   region, block_size);
+	} else {
+		ret = onenand_multiblock_erase(mtd, instr, block_size);
+	}
 
 	/* Deselect and wake up anyone waiting on the device */
 	onenand_release_device(mtd);
diff --git a/include/linux/mtd/flashchip.h b/include/linux/mtd/flashchip.h
index f350a48..d0bf422 100644
--- a/include/linux/mtd/flashchip.h
+++ b/include/linux/mtd/flashchip.h
@@ -41,9 +41,11 @@ typedef enum {
 	/* These 2 come from nand_state_t, which has been unified here */
 	FL_READING,
 	FL_CACHEDPRG,
-	/* These 2 come from onenand_state_t, which has been unified here */
+	/* These 4 come from onenand_state_t, which has been unified here */
 	FL_RESETING,
 	FL_OTPING,
+	FL_PREPARING_ERASE,
+	FL_VERIFYING_ERASE,
 
 	FL_UNKNOWN
 } flstate_t;
diff --git a/include/linux/mtd/onenand_regs.h b/include/linux/mtd/onenand_regs.h
index acadbf5..cd6f3b4 100644
--- a/include/linux/mtd/onenand_regs.h
+++ b/include/linux/mtd/onenand_regs.h
@@ -131,6 +131,8 @@
 #define ONENAND_CMD_LOCK_TIGHT		(0x2C)
 #define ONENAND_CMD_UNLOCK_ALL		(0x27)
 #define ONENAND_CMD_ERASE		(0x94)
+#define ONENAND_CMD_MULTIBLOCK_ERASE	(0x95)
+#define ONENAND_CMD_ERASE_VERIFY	(0x71)
 #define ONENAND_CMD_RESET		(0xF0)
 #define ONENAND_CMD_OTP_ACCESS		(0x65)
 #define ONENAND_CMD_READID		(0x90)
-- 
1.6.0.4

[PATCH v5 3/3] MTD: OneNAND: fix double printing of function name

[Mika Korhonen]

Signed-off-by: Mika Korhonen <ext-mika.2.korhonen@nokia.com>
---
 drivers/mtd/onenand/onenand_base.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 7355f62..e812552 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -3468,8 +3468,8 @@ int flexonenand_set_boundary(struct mtd_info *mtd, int die,
 	this->command(mtd, ONENAND_CMD_ERASE, addr, 0);
 	ret = this->wait(mtd, FL_ERASING);
 	if (ret) {
-		printk(KERN_ERR "%s: flexonenand_set_boundary: "
-				"Failed PI erase for Die %d\n", __func__, die);
+		printk(KERN_ERR "%s: Failed PI erase for Die %d\n",
+		       __func__, die);
 		goto out;
 	}
 
-- 
1.6.0.4

Re: [PATCH v5 0/3] MTD: OneNAND: multiblock erase support

[Artem Bityutskiy]

Similarly, waiting for Adrian to review this. He said he'll do soon.

On Mon, 2009-10-12 at 12:52 +0300, Mika Korhonen wrote:
> (v5) Adjust patches to apply on current tree. Use __func__ in printk's.
>      Fix mistakes found by K. Park.
> 
> (v4) This patch series is a reworked version of
> 
> http://lists.infradead.org/pipermail/linux-mtd/2009-September/027138.html
> http://lists.infradead.org/pipermail/linux-mtd/2009-September/027139.html
> http://lists.infradead.org/pipermail/linux-mtd/2009-September/027140.html
> 
> based on comments by Adrian Hunter and Kyungmin Park. This version should
> address the issues pointed out by them.
> 
> The original patch is:
> http://lists.infradead.org/pipermail/linux-mtd/2009-June/026130.html
> 
> I split the original patch in two for readability: the first part extracts
> the execution of erase command to a separate function for easier integration
> of different erase method. The second part implements the multiblock erase
> function in case multiple blocks are requested to be erased and the chip is
> not Flex.
> 
> This is useful for flashing applications that need to do their work as
> fast as possible. For full 64 eraseblock case the erase speed is up to 30x
> faster. (Samsung: 64 MB/s vs 2.1 GB/s, I got 1.4 GB/s on Linux kernel)
> 
> 
> Mika Korhonen (3):
>   MTD: OneNAND: move erase method to a separate function
>   MTD: OneNAND: multiblock erase support
>   MTD: OneNAND: fix double printing of function name
> 
>  drivers/mtd/onenand/omap2.c        |   22 +++-
>  drivers/mtd/onenand/onenand_base.c |  289 +++++++++++++++++++++++++++++------
>  include/linux/mtd/flashchip.h      |    4 +-
>  include/linux/mtd/onenand_regs.h   |    2 +
>  4 files changed, 262 insertions(+), 55 deletions(-)
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH v5 2/3] MTD: OneNAND: multiblock erase support

[Adrian Hunter]

Korhonen Mika.2 (EXT-Ardites/Oulu) wrote:
> Add support for multiblock erase command. OneNANDs (excluding Flex-OneNAND)
> are capable of simultaneous erase of up to 64 eraseblocks which is much faster.
> 
> This changes the erase requests for regions covering multiple eraseblocks
> to be performed using multiblock erase.
> 
> Signed-off-by: Mika Korhonen <ext-mika.2.korhonen@nokia.com>
> ---

Just a comment about fail_addr

>  drivers/mtd/onenand/omap2.c        |   22 ++++-
>  drivers/mtd/onenand/onenand_base.c |  173 +++++++++++++++++++++++++++++++++++-
>  include/linux/mtd/flashchip.h      |    4 +-
>  include/linux/mtd/onenand_regs.h   |    2 +
>  4 files changed, 194 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/onenand/omap2.c b/drivers/mtd/onenand/omap2.c
> index 0108ed4..2dafd09 100644
> --- a/drivers/mtd/onenand/omap2.c
> +++ b/drivers/mtd/onenand/omap2.c
> @@ -112,10 +112,24 @@ static int omap2_onenand_wait(struct mtd_info *mtd, int state)
>  	unsigned long timeout;
>  	u32 syscfg;
>  
> -	if (state == FL_RESETING) {
> -		int i;
> +	if (state == FL_RESETING || state == FL_PREPARING_ERASE ||
> +	    state == FL_VERIFYING_ERASE) {
> +		int i = 21;
> +		unsigned int intr_flags = ONENAND_INT_MASTER;
> +
> +		switch (state) {
> +		case FL_RESETING:
> +			intr_flags |= ONENAND_INT_RESET;
> +			break;
> +		case FL_PREPARING_ERASE:
> +			intr_flags |= ONENAND_INT_ERASE;
> +			break;
> +		case FL_VERIFYING_ERASE:
> +			i = 101;
> +			break;
> +		}
>  
> -		for (i = 0; i < 20; i++) {
> +		while (--i) {
>  			udelay(1);
>  			intr = read_reg(c, ONENAND_REG_INTERRUPT);
>  			if (intr & ONENAND_INT_MASTER)
> @@ -126,7 +140,7 @@ static int omap2_onenand_wait(struct mtd_info *mtd, int state)
>  			wait_err("controller error", state, ctrl, intr);
>  			return -EIO;
>  		}
> -		if (!(intr & ONENAND_INT_RESET)) {
> +		if ((intr & intr_flags) != intr_flags) {
>  			wait_err("timeout", state, ctrl, intr);
>  			return -EIO;
>  		}
> diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
> index 51f4782..7355f62 100644
> --- a/drivers/mtd/onenand/onenand_base.c
> +++ b/drivers/mtd/onenand/onenand_base.c
> @@ -32,6 +32,13 @@
>  
>  #include <asm/io.h>
>  
> +/*
> + * Multiblock erase if number of blocks to erase is 2 or more.
> + * Maximum number of blocks for simultaneous erase is 64.
> + */
> +#define MB_ERASE_MIN_BLK_COUNT 2
> +#define MB_ERASE_MAX_BLK_COUNT 64
> +
>  /* Default Flex-OneNAND boundary and lock respectively */
>  static int flex_bdry[MAX_DIES * 2] = { -1, 0, -1, 0 };
>  
> @@ -339,6 +346,8 @@ static int onenand_command(struct mtd_info *mtd, int cmd, loff_t addr, size_t le
>  		break;
>  
>  	case ONENAND_CMD_ERASE:
> +	case ONENAND_CMD_MULTIBLOCK_ERASE:
> +	case ONENAND_CMD_ERASE_VERIFY:
>  	case ONENAND_CMD_BUFFERRAM:
>  	case ONENAND_CMD_OTP_ACCESS:
>  		block = onenand_block(this, addr);
> @@ -483,7 +492,7 @@ static int onenand_wait(struct mtd_info *mtd, int state)
>  		if (interrupt & flags)
>  			break;
>  
> -		if (state != FL_READING)
> +		if (state != FL_READING && state != FL_PREPARING_ERASE)
>  			cond_resched();
>  	}
>  	/* To get correct interrupt status in timeout case */
> @@ -516,6 +525,18 @@ static int onenand_wait(struct mtd_info *mtd, int state)
>  		return -EIO;
>  	}
>  
> +	if (state == FL_PREPARING_ERASE && !(interrupt & ONENAND_INT_ERASE)) {
> +		printk(KERN_ERR "%s: mb erase timeout! ctrl=0x%04x intr=0x%04x\n",
> +		       __func__, ctrl, interrupt);
> +		return -EIO;
> +	}
> +
> +	if (!(interrupt & ONENAND_INT_MASTER)) {
> +		printk(KERN_ERR "%s: timeout! ctrl=0x%04x intr=0x%04x\n",
> +		       __func__, ctrl, interrupt);
> +		return -EIO;
> +	}
> +
>  	/* If there's controller error, it's a real error */
>  	if (ctrl & ONENAND_CTRL_ERROR) {
>  		printk(KERN_ERR "%s: controller error = 0x%04x\n",
> @@ -2168,6 +2189,148 @@ static int onenand_block_isbad_nolock(struct mtd_info *mtd, loff_t ofs, int allo
>  	return bbm->isbad_bbt(mtd, ofs, allowbbt);
>  }
>  
> +
> +static int onenand_multiblock_erase_verify(struct mtd_info *mtd,
> +					   struct erase_info *instr)
> +{
> +	struct onenand_chip *this = mtd->priv;
> +	loff_t addr = instr->addr;
> +	int len = instr->len;
> +	unsigned int block_size = (1 << this->erase_shift);
> +	int ret = 0;
> +
> +	while (len) {
> +		this->command(mtd, ONENAND_CMD_ERASE_VERIFY, addr, block_size);
> +		ret = this->wait(mtd, FL_VERIFYING_ERASE);
> +		if (ret) {
> +			printk(KERN_ERR "%s: Failed verify, block %d\n",
> +			       __func__, onenand_block(this, addr));
> +			instr->state = MTD_ERASE_FAILED;
> +			instr->fail_addr = addr;
> +			return -1;
> +		}
> +		len -= block_size;
> +		addr += block_size;
> +	}
> +	return 0;
> +}
> +
> +/**
> + * onenand_multiblock_erase - [Internal] erase block(s) using multiblock erase
> + * @param mtd		MTD device structure
> + * @param instr		erase instruction
> + * @param region	erase region
> + *
> + * Erase one or more blocks up to 64 block at a time
> + */
> +static int onenand_multiblock_erase(struct mtd_info *mtd,
> +				    struct erase_info *instr,
> +				    unsigned int block_size)
> +{
> +	struct onenand_chip *this = mtd->priv;
> +	loff_t addr = instr->addr;
> +	int len = instr->len;
> +	int eb_count = 0;
> +	int ret = 0;
> +	int bdry_block = 0;
> +
> +	instr->state = MTD_ERASING;
> +
> +	if (ONENAND_IS_DDP(this)) {
> +		loff_t bdry_addr = this->chipsize >> 1;
> +		if (addr < bdry_addr && (addr + len) > bdry_addr)
> +			bdry_block = bdry_addr >> this->erase_shift;
> +	}
> +
> +	/* Pre-check bbs */
> +	while (len) {
> +		/* Check if we have a bad block, we do not erase bad blocks */
> +		if (onenand_block_isbad_nolock(mtd, addr, 0)) {
> +			printk(KERN_WARNING "%s: attempt to erase a bad block "
> +			       "at addr 0x%012llx\n",
> +			       __func__, (unsigned long long) addr);
> +			instr->state = MTD_ERASE_FAILED;
> +			return -EIO;
> +		}
> +		len -= block_size;
> +		addr += block_size;
> +	}
> +
> +	len = instr->len;
> +	addr = instr->addr;
> +
> +	/* loop over 64 eb batches */
> +	while (len) {
> +		struct erase_info verify_instr = *instr;
> +		int max_eb_count = MB_ERASE_MAX_BLK_COUNT;
> +
> +		verify_instr.addr = addr;
> +		verify_instr.len = 0;
> +
> +		/* do not cross chip boundary */
> +		if (bdry_block) {
> +			int this_block = (addr >> this->erase_shift);
> +
> +			if (this_block < bdry_block) {
> +				max_eb_count = min(max_eb_count,
> +						   (bdry_block - this_block));
> +			}
> +		}
> +
> +		eb_count = 0;
> +
> +		while (len > block_size && eb_count < (max_eb_count - 1)) {
> +			this->command(mtd, ONENAND_CMD_MULTIBLOCK_ERASE,
> +				      addr, block_size);
> +			onenand_invalidate_bufferram(mtd, addr, block_size);
> +
> +			ret = this->wait(mtd, FL_PREPARING_ERASE);
> +			if (ret) {
> +				printk(KERN_ERR "%s: Failed multiblock erase, "
> +				       "block %d\n", __func__,
> +				       onenand_block(this, addr));
> +				instr->state = MTD_ERASE_FAILED;
> +				instr->fail_addr = addr;

This might lead a caller to believe all eraseblocks before fail_addr have been erased.
Better to leave it MTD_FAIL_ADDR_UNKNOWN in this case.

> +				return -EIO;
> +			}
> +
> +			len -= block_size;
> +			addr += block_size;
> +			eb_count++;
> +		}
> +
> +		/* last block of 64-eb series */
> +		cond_resched();
> +		this->command(mtd, ONENAND_CMD_ERASE, addr, block_size);
> +		onenand_invalidate_bufferram(mtd, addr, block_size);
> +
> +		ret = this->wait(mtd, FL_ERASING);
> +		/* Check if it is write protected */
> +		if (ret) {
> +			printk(KERN_ERR "%s: Failed erase, block %d\n",
> +			       __func__, onenand_block(this, addr));
> +			instr->state = MTD_ERASE_FAILED;
> +			instr->fail_addr = addr;

Ditto

> +			return -EIO;
> +		}
> +
> +		len -= block_size;
> +		addr += block_size;
> +		eb_count++;
> +
> +		/* verify */
> +		verify_instr.len = eb_count * block_size;
> +		if (onenand_multiblock_erase_verify(mtd, &verify_instr)) {
> +			instr->state = verify_instr.state;
> +			instr->fail_addr = verify_instr.fail_addr;
> +			return -EIO;
> +		}
> +
> +	}
> +	return 0;
> +}
> +
> +
>  /**
>   * onenand_block_by_block_erase - [Internal] erase block(s) using regular erase
>   * @param mtd		MTD device structure
> @@ -2301,7 +2464,13 @@ static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
>  	/* Grab the lock and see if the device is available */
>  	onenand_get_device(mtd, FL_ERASING);
>  
> -	ret = onenand_block_by_block_erase(mtd, instr, region, block_size);
> +	if (region || instr->len < MB_ERASE_MIN_BLK_COUNT * block_size) {
> +		/* region is set for Flex-OneNAND (no mb erase) */
> +		ret = onenand_block_by_block_erase(mtd, instr,
> +						   region, block_size);
> +	} else {
> +		ret = onenand_multiblock_erase(mtd, instr, block_size);
> +	}
>  
>  	/* Deselect and wake up anyone waiting on the device */
>  	onenand_release_device(mtd);
> diff --git a/include/linux/mtd/flashchip.h b/include/linux/mtd/flashchip.h
> index f350a48..d0bf422 100644
> --- a/include/linux/mtd/flashchip.h
> +++ b/include/linux/mtd/flashchip.h
> @@ -41,9 +41,11 @@ typedef enum {
>  	/* These 2 come from nand_state_t, which has been unified here */
>  	FL_READING,
>  	FL_CACHEDPRG,
> -	/* These 2 come from onenand_state_t, which has been unified here */
> +	/* These 4 come from onenand_state_t, which has been unified here */
>  	FL_RESETING,
>  	FL_OTPING,
> +	FL_PREPARING_ERASE,
> +	FL_VERIFYING_ERASE,
>  
>  	FL_UNKNOWN
>  } flstate_t;
> diff --git a/include/linux/mtd/onenand_regs.h b/include/linux/mtd/onenand_regs.h
> index acadbf5..cd6f3b4 100644
> --- a/include/linux/mtd/onenand_regs.h
> +++ b/include/linux/mtd/onenand_regs.h
> @@ -131,6 +131,8 @@
>  #define ONENAND_CMD_LOCK_TIGHT		(0x2C)
>  #define ONENAND_CMD_UNLOCK_ALL		(0x27)
>  #define ONENAND_CMD_ERASE		(0x94)
> +#define ONENAND_CMD_MULTIBLOCK_ERASE	(0x95)
> +#define ONENAND_CMD_ERASE_VERIFY	(0x71)
>  #define ONENAND_CMD_RESET		(0xF0)
>  #define ONENAND_CMD_OTP_ACCESS		(0x65)
>  #define ONENAND_CMD_READID		(0x90)

Re: [PATCH v5 2/3] MTD: OneNAND: multiblock erase support

[Artem Bityutskiy]

On Tue, 2009-10-20 at 16:09 +0300, Adrian Hunter wrote:
> Korhonen Mika.2 (EXT-Ardites/Oulu) wrote:
> > Add support for multiblock erase command. OneNANDs (excluding Flex-OneNAND)
> > are capable of simultaneous erase of up to 64 eraseblocks which is much faster.
> > 
> > This changes the erase requests for regions covering multiple eraseblocks
> > to be performed using multiblock erase.
> > 
> > Signed-off-by: Mika Korhonen <ext-mika.2.korhonen@nokia.com>
> > ---
> 
> Just a comment about fail_addr

Mika, are you planning to amend this and re-send?

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

Re: [PATCH v5 2/3] MTD: OneNAND: multiblock erase support

[Mika Korhonen]

Bityutskiy Artem (Nokia-D/Helsinki) wrote:
> On Tue, 2009-10-20 at 16:09 +0300, Adrian Hunter wrote:
>   
>> Korhonen Mika.2 (EXT-Ardites/Oulu) wrote:
>>     
>>> Add support for multiblock erase command. OneNANDs (excluding Flex-OneNAND)
>>> are capable of simultaneous erase of up to 64 eraseblocks which is much faster.
>>>
>>> This changes the erase requests for regions covering multiple eraseblocks
>>> to be performed using multiblock erase.
>>>
>>> Signed-off-by: Mika Korhonen <ext-mika.2.korhonen@nokia.com>
>>> ---
>>>       
>> Just a comment about fail_addr
>>     
>
> Mika, are you planning to amend this and re-send?
>
>   
Yes, sorry been quite busy  lately. Shall I resend this very patch with 
Adrian's improvements to this thread only or mail the whole set again.

Mika

Re: [PATCH v5 2/3] MTD: OneNAND: multiblock erase support

[Artem Bityutskiy]

On Fri, 2009-10-23 at 07:19 +0200, Korhonen Mika.2 (EXT-Ardites/Oulu)
wrote:
> Bityutskiy Artem (Nokia-D/Helsinki) wrote:
> > On Tue, 2009-10-20 at 16:09 +0300, Adrian Hunter wrote:
> >   
> >> Korhonen Mika.2 (EXT-Ardites/Oulu) wrote:
> >>     
> >>> Add support for multiblock erase command. OneNANDs (excluding Flex-OneNAND)
> >>> are capable of simultaneous erase of up to 64 eraseblocks which is much faster.
> >>>
> >>> This changes the erase requests for regions covering multiple eraseblocks
> >>> to be performed using multiblock erase.
> >>>
> >>> Signed-off-by: Mika Korhonen <ext-mika.2.korhonen@nokia.com>
> >>> ---
> >>>       
> >> Just a comment about fail_addr
> >>     
> >
> > Mika, are you planning to amend this and re-send?
> >
> >   
> Yes, sorry been quite busy  lately. Shall I resend this very patch with 
> Adrian's improvements to this thread only or mail the whole set again.

Up to you. I can pick them up both ways.

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

Re: [PATCH v5 2/3] MTD: OneNAND: multiblock erase support

[Amit Kumar Sharma]

Hi Mika,

Do you have any use case for using multiblock erase.


Thanks
Amit
----- Original Message ----- 
From: "Artem Bityutskiy" <Artem.Bityutskiy@nokia.com>
To: "Korhonen Mika.2 (EXT-Ardites/Oulu)" 
<ext-mika.2.korhonen@nokia.com>
Cc: <amul.saha@samsung.com>; <kyungmin.park@samsung.com>; 
<linux-mtd@lists.infradead.org>; "Adrian Hunter" 
<adrian.hunter@nokia.com>
Sent: Friday, October 23, 2009 10:04 AM
Subject: Re: [PATCH v5 2/3] MTD: OneNAND: multiblock erase 
support


> On Tue, 2009-10-20 at 16:09 +0300, Adrian Hunter wrote:
>> Korhonen Mika.2 (EXT-Ardites/Oulu) wrote:
>> > Add support for multiblock erase command. OneNANDs 
>> > (excluding Flex-OneNAND)
>> > are capable of simultaneous erase of up to 64 
>> > eraseblocks which is much faster.
>> >
>> > This changes the erase requests for regions covering 
>> > multiple eraseblocks
>> > to be performed using multiblock erase.
>> >
>> > Signed-off-by: Mika Korhonen 
>> > <ext-mika.2.korhonen@nokia.com>
>> > ---
>>
>> Just a comment about fail_addr
>
> Mika, are you planning to amend this and re-send?
>
> -- 
> Best Regards,
> Artem Bityutskiy (????? ????????)
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

Re: [PATCH v5 2/3] MTD: OneNAND: multiblock erase support

[Artem Bityutskiy]

On Fri, 2009-10-23 at 07:41 +0200, ext Amit Kumar Sharma wrote:
> Hi Mika,
> 
> Do you have any use case for using multiblock erase.

One use case I see is flashing UBI images faster. With some tweaks, the
ubiformat utility could exploit the mutiblock-erase feature (it needs to
erase the mtd partition before flashing).

Well, to be generic, it would be also nice to have a sysfs file telling
the maximum multi-block erase size. However, it is not strictly
necessary.

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

Re: [PATCH v5 2/3] MTD: OneNAND: multiblock erase support

[Mika Korhonen]

ext Amit Kumar Sharma wrote:
> Hi Mika,
>
> Do you have any use case for using multiblock erase.
>
>
> Thanks
> Amit
>   
Yes, e.g. during development/loop testing we need to flash a new system 
on an MTD partition.
The (UBIFS) system could be tiny but the partition large in which case 
the erase time becomes even more significant.
Even in more usual cases of reflashing a bigger system this gives ca. 5 
% speed improvement.

Mika

Re: [PATCH v5 2/3] MTD: OneNAND: multiblock erase support

[Adrian Hunter]

Korhonen Mika.2 (EXT-Ardites/Oulu) wrote:
> ext Amit Kumar Sharma wrote:
>> Hi Mika,
>>
>> Do you have any use case for using multiblock erase.
>>
>>
>> Thanks
>> Amit
>>   
> Yes, e.g. during development/loop testing we need to flash a new system 
> on an MTD partition.
> The (UBIFS) system could be tiny but the partition large in which case 
> the erase time becomes even more significant.
> Even in more usual cases of reflashing a bigger system this gives ca. 5 
> % speed improvement.
> 
> Mika
> 

However the API is imperfect because AFAICT OneNAND multiblock erase
does not require that the blocks are sequential i.e. any 64 eraseblocks
from the same chip can be erased together.  So we would really need an
'erasev()' API to take complete advantage of it.

Re: [PATCH v5 2/3] MTD: OneNAND: multiblock erase support

[Amit Kumar Sharma]

> Korhonen Mika.2 (EXT-Ardites/Oulu) wrote:
>> ext Amit Kumar Sharma wrote:
>>> Hi Mika,
>>>
>>> Do you have any use case for using multiblock erase.
>>>
>>>
>>> Thanks
>>> Amit
>>>
>> Yes, e.g. during development/loop testing we need to 
>> flash a new system on an MTD partition.
>> The (UBIFS) system could be tiny but the partition large 
>> in which case the erase time becomes even more 
>> significant.
>> Even in more usual cases of reflashing a bigger system 
>> this gives ca. 5 % speed improvement.
>>
>> Mika
>>
>
> However the API is imperfect because AFAICT OneNAND 
> multiblock erase
> does not require that the blocks are sequential i.e. any 
> 64 eraseblocks
> from the same chip can be erased together.  So we would 
> really need an
> 'erasev()' API to take complete advantage of it.
>
I feel we should have a use case for multi block erase for 
UBI wear leveling.
it is just a opinion.
Multi block erase can  be useful if implement 'erasev' api 
at file system level.

Thanks
Amit