[PATCH u-boot-spi v2 0/9] Fix `mtd erase` when used with mtdpart

[PATCH u-boot-spi v2 0/9] Fix `mtd erase` when used with mtdpart

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

The original cover letter said:

this patch series fixes the `mtd erase` command when used with mtdpart
with a partition of non-zero offset.

Currently when the `mtd erase` command is used for such a partition,
it does not erase all blocks. Instead after a block is erased, the next
block address not current block address + block size, but current block
address + block size + partition offset, due to spi_nor_erase() not
calling mtd_erase_callback():
  => mtd erase "Rescue system"  
  Erasing 0x00000000 ... 0x006fffff (1792 eraseblock(s))
  jedec_spi_nor spi-nor@0: at 0x100000, len 4096
  jedec_spi_nor spi-nor@0: at 0x201000, len 4096
  jedec_spi_nor spi-nor@0: at 0x302000, len 4096
  jedec_spi_nor spi-nor@0: at 0x403000, len 4096
  jedec_spi_nor spi-nor@0: at 0x504000, len 4096
  jedec_spi_nor spi-nor@0: at 0x605000, len 4096
  jedec_spi_nor spi-nor@0: at 0x706000, len 4096

This series adds some fixes to spi_nor_erase() function, then adds
calling of mtd_erase_callback() to fix this bug.

The series also contains an improvement - adding the posibility to
interrupt spi_nor_erase() with Ctrl+C; and another one - making mtdpart's
_erase() method more sane so that the above mentioned bug will not occur
even if underlying driver does not call mtd_erase_callback().

Finally the last patch removes mtd_erase_callback() entirely, since:
- all provided callbacks across U-Boot are no-ops
- mtd_erase_callback() is abused for completely different purpose
  than the original one (as explained in last commit message)

Marek

Changes since v1:
- fixed CI bugs (by removing mtd_erase_callback() entirely)

Marek Behún (9):
  mtd: spi-nor-core: Try cleaning up in case writing BAR failed
  mtd: spi-nor-core: Check return value of write_enable() in
    spi_nor_erase()
  mtd: spi-nor-core: Don't overwrite return value if it is non-zero
  mtd: spi-nor-core: Check return value of write_disable() in
    spi_nor_erase()
  mtd: spi-nor-core: Don't check for zero length in spi_nor_erase()
  mtd: spi-nor-core: Call mtd_erase_callback() from spi_nor_erase()
  mtd: spi-nor-core: Check for ctrlc() in spi_nor_erase()
  mtd: mtdpart: Make mtdpart's _erase method sane
  mtd: Remove mtd_erase_callback() entirely

 cmd/onenand.c                      |  9 ++-----
 drivers/mtd/altera_qspi.c          |  3 ---
 drivers/mtd/cfi_mtd.c              |  1 -
 drivers/mtd/mtdconcat.c            | 11 --------
 drivers/mtd/mtdcore.c              |  8 ------
 drivers/mtd/mtdpart.c              | 23 +++++-----------
 drivers/mtd/nand/raw/nand_base.c   |  4 ---
 drivers/mtd/onenand/onenand_base.c |  3 ---
 drivers/mtd/spi/sf_mtd.c           |  1 -
 drivers/mtd/spi/spi-nor-core.c     | 43 +++++++++++++++++++++++-------
 drivers/mtd/ubi/io.c               | 13 ---------
 env/onenand.c                      |  4 +--
 fs/yaffs2/yaffs_mtdif.c            |  1 -
 include/linux/mtd/mtd.h            | 11 --------
 include/nand.h                     |  1 -
 15 files changed, 42 insertions(+), 94 deletions(-)

-- 
2.32.0

[PATCH u-boot-spi v2 1/9] mtd: spi-nor-core: Try cleaning up in case writing BAR failed

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

Use the cleanup codepath of spi_nor_erase() also in the event of failure
of writing the BAR register.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 drivers/mtd/spi/spi-nor-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index d5d905fa5a..529e7e9178 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -927,7 +927,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 #ifdef CONFIG_SPI_FLASH_BAR
 		ret = write_bar(nor, addr);
 		if (ret < 0)
-			return ret;
+			goto erase_err;
 #endif
 		write_enable(nor);
 
-- 
2.32.0

[PATCH u-boot-spi v2 2/9] mtd: spi-nor-core: Check return value of write_enable() in spi_nor_erase()

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

The spi_nor_erase() function does not check return value of the
write_enable() call. Fix this.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 drivers/mtd/spi/spi-nor-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 529e7e9178..d2018ab702 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -929,7 +929,9 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 		if (ret < 0)
 			goto erase_err;
 #endif
-		write_enable(nor);
+		ret = write_enable(nor);
+		if (ret < 0)
+			goto erase_err;
 
 		ret = spi_nor_erase_sector(nor, addr);
 		if (ret < 0)
-- 
2.32.0

[PATCH u-boot-spi v2 3/9] mtd: spi-nor-core: Don't overwrite return value if it is non-zero

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

The cleanup code of the spi_nor_erase() function overwrites the ret
variable with return value of clean_bar(), even if the ret variable is
already set. Fix this.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 drivers/mtd/spi/spi-nor-core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index d2018ab702..30c0afc08c 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -907,7 +907,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
 	struct spi_nor *nor = mtd_to_spi_nor(mtd);
 	u32 addr, len, rem;
-	int ret;
+	int ret, err;
 
 	dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr,
 		(long long)instr->len);
@@ -947,7 +947,9 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 
 erase_err:
 #ifdef CONFIG_SPI_FLASH_BAR
-	ret = clean_bar(nor);
+	err = clean_bar(nor);
+	if (!ret)
+		ret = err;
 #endif
 	write_disable(nor);
 
-- 
2.32.0

[PATCH u-boot-spi v2 4/9] mtd: spi-nor-core: Check return value of write_disable() in spi_nor_erase()

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

The cleanup code of spi_nor_erase() function calls write_disable(), but
does not return it's return value even in case of failure. Fix this.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 drivers/mtd/spi/spi-nor-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 30c0afc08c..9e936cbe1a 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -951,7 +951,9 @@ erase_err:
 	if (!ret)
 		ret = err;
 #endif
-	write_disable(nor);
+	err = write_disable(nor);
+	if (!ret)
+		ret = err;
 
 	return ret;
 }
-- 
2.32.0

[PATCH u-boot-spi v2 5/9] mtd: spi-nor-core: Don't check for zero length in spi_nor_erase()

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

This check is already done in mtdcore's mtd_erase(), no reason to do
this here as well.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 drivers/mtd/spi/spi-nor-core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 9e936cbe1a..211eea22a4 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -912,9 +912,6 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 	dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr,
 		(long long)instr->len);
 
-	if (!instr->len)
-		return 0;
-
 	div_u64_rem(instr->len, mtd->erasesize, &rem);
 	if (rem)
 		return -EINVAL;
-- 
2.32.0

[PATCH u-boot-spi v2 6/9] mtd: spi-nor-core: Call mtd_erase_callback() from spi_nor_erase()

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

The spi_nor_erase() function does not call mtd_erase_callback() as it
should.

The mtdpart code currently implements the subtraction of partition
offset in mtd_erase_callback().

This results in partition offset being added prior calling
spi_nor_erase(), but not subtracted back on return. The result is that
the `mtd erase` command does not erase the whole partition, only some of
it's blocks:

  => mtd erase "Rescue system"
  Erasing 0x00000000 ... 0x006fffff (1792 eraseblock(s))
  jedec_spi_nor spi-nor@0: at 0x100000, len 4096
  jedec_spi_nor spi-nor@0: at 0x201000, len 4096
  jedec_spi_nor spi-nor@0: at 0x302000, len 4096
  jedec_spi_nor spi-nor@0: at 0x403000, len 4096
  jedec_spi_nor spi-nor@0: at 0x504000, len 4096
  jedec_spi_nor spi-nor@0: at 0x605000, len 4096
  jedec_spi_nor spi-nor@0: at 0x706000, len 4096

This is obviously wrong.

Add proper calling of mtd_erase_callback() into the spi_nor_erase()
function.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reported-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 drivers/mtd/spi/spi-nor-core.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 211eea22a4..f3e08ed412 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -906,6 +906,7 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
 static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
 	struct spi_nor *nor = mtd_to_spi_nor(mtd);
+	bool addr_known = false;
 	u32 addr, len, rem;
 	int ret, err;
 
@@ -913,12 +914,17 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 		(long long)instr->len);
 
 	div_u64_rem(instr->len, mtd->erasesize, &rem);
-	if (rem)
-		return -EINVAL;
+	if (rem) {
+		ret = -EINVAL;
+		goto erase_err_callback;
+	}
 
 	addr = instr->addr;
 	len = instr->len;
 
+	instr->state = MTD_ERASING;
+	addr_known = true;
+
 	while (len) {
 		WATCHDOG_RESET();
 #ifdef CONFIG_SPI_FLASH_BAR
@@ -942,6 +948,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 			goto erase_err;
 	}
 
+	addr_known = false;
 erase_err:
 #ifdef CONFIG_SPI_FLASH_BAR
 	err = clean_bar(nor);
@@ -952,6 +959,15 @@ erase_err:
 	if (!ret)
 		ret = err;
 
+erase_err_callback:
+	if (ret) {
+		instr->fail_addr = addr_known ? addr : MTD_FAIL_ADDR_UNKNOWN;
+		instr->state = MTD_ERASE_FAILED;
+	} else {
+		instr->state = MTD_ERASE_DONE;
+	}
+	mtd_erase_callback(instr);
+
 	return ret;
 }
 
-- 
2.32.0

[PATCH u-boot-spi v2 7/9] mtd: spi-nor-core: Check for ctrlc() in spi_nor_erase()

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

May it possible to interrupt the spi_nor_erase() function.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 drivers/mtd/spi/spi-nor-core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index f3e08ed412..124594f0cd 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -927,6 +927,11 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 
 	while (len) {
 		WATCHDOG_RESET();
+		if (ctrlc()) {
+			addr_known = false;
+			ret = -EINTR;
+			goto erase_err;
+		}
 #ifdef CONFIG_SPI_FLASH_BAR
 		ret = write_bar(nor, addr);
 		if (ret < 0)
-- 
2.32.0

[PATCH u-boot-spi v2 8/9] mtd: mtdpart: Make mtdpart's _erase method sane

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

The _erase() method of the mtdpart driver, part_erase(), currently
implements offset shifting (for given mtdpart partition) in a weird way:
  1. part_erase() adds partition offset to block address
  2. parent driver's _erase() method is called
  3. parent driver's _erase() method calls mtd_erase_callback()
  4. mtd_erase_callback() subtracts partition offset from block address
     so that the callback function is given correct address
The problem here is that if the parent's driver does not call
mtd_erase_callback() in some scenario (this was recently a case for
spi_nor_erase(), which did not call mtd_erase_callback() at all), the
offset is not shifted back.

Moreover the code would be more readable if part_erase() not only added
partition offset before calling parent's _erase(), but also subtracted
it back afterwards. Currently the mtd_erase_callback() is expected to do
this subtracting since it does have to do it anyway.

Add the more steps to this procedure:
  5. mtd_erase_callback() adds partition offset to block address so that
     it returns the the erase_info structure members as it received them
  6. part_erase() subtracts partition offset from block address

This makes the code more logical and also prevents errors in case
parent's driver does not call mtd_erase_callback() for some reason.

(BTW, the purpose of mtd_erase_callback() in Linux is to inform the
 caller that it is done, since in Linux erasing is done asynchronously.
 We are abusing the purpose of mtd_erase_callback() in U-Boot for
 completely different purpose. The callback function itself has empty
 implementation in all cases in U-Boot.)

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 drivers/mtd/mtdpart.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index aa58f722da..6ab481a7b1 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -446,24 +446,34 @@ static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
 	int ret;
 
 	instr->addr += mtd->offset;
+
 	ret = mtd->parent->_erase(mtd->parent, instr);
-	if (ret) {
-		if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
-			instr->fail_addr -= mtd->offset;
-		instr->addr -= mtd->offset;
-	}
+	if (ret && instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
+		instr->fail_addr -= mtd->offset;
+
+	instr->addr -= mtd->offset;
+
 	return ret;
 }
 
 void mtd_erase_callback(struct erase_info *instr)
 {
-	if (instr->mtd->_erase == part_erase) {
+	if (!instr->callback)
+		return;
+
+	if (instr->mtd->_erase == part_erase && instr->len) {
 		if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
 			instr->fail_addr -= instr->mtd->offset;
 		instr->addr -= instr->mtd->offset;
 	}
-	if (instr->callback)
-		instr->callback(instr);
+
+	instr->callback(instr);
+
+	if (instr->mtd->_erase == part_erase && instr->len) {
+		if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
+			instr->fail_addr += instr->mtd->offset;
+		instr->addr += instr->mtd->offset;
+	}
 }
 EXPORT_SYMBOL_GPL(mtd_erase_callback);
 
-- 
2.32.0

[PATCH u-boot-spi v2 9/9] mtd: Remove mtd_erase_callback() entirely

[Marek Behún]

From: Marek Behún <marek.behun@nic.cz>

The original purpose of mtd_erase_callback() in Linux at the time it was
imported to U-Boot, was to inform the caller that erasing is done (since
it was an asynchronous operation).

All supplied callback methods in U-Boot do nothing, but the
mtd_erase_callback() function was (until previous patch) grossly abused
in U-Boot's mtdpart implementation for completely different purpose.

Since we got rid of the abusement, remove the mtd_erase_callback()
function and the .callback member from struct erase_info entirely, in
order to avoid such problems in the future.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 cmd/onenand.c                      |  9 ++-------
 drivers/mtd/altera_qspi.c          |  3 ---
 drivers/mtd/cfi_mtd.c              |  1 -
 drivers/mtd/mtdconcat.c            | 11 -----------
 drivers/mtd/mtdcore.c              |  8 --------
 drivers/mtd/mtdpart.c              | 21 ---------------------
 drivers/mtd/nand/raw/nand_base.c   |  4 ----
 drivers/mtd/onenand/onenand_base.c |  3 ---
 drivers/mtd/spi/sf_mtd.c           |  1 -
 drivers/mtd/spi/spi-nor-core.c     |  5 ++---
 drivers/mtd/ubi/io.c               | 13 -------------
 env/onenand.c                      |  4 +---
 fs/yaffs2/yaffs_mtdif.c            |  1 -
 include/linux/mtd/mtd.h            | 11 -----------
 include/nand.h                     |  1 -
 15 files changed, 5 insertions(+), 91 deletions(-)

diff --git a/cmd/onenand.c b/cmd/onenand.c
index 852ed5c7b2..592985a7ee 100644
--- a/cmd/onenand.c
+++ b/cmd/onenand.c
@@ -186,9 +186,7 @@ next:
 static int onenand_block_erase(u32 start, u32 size, int force)
 {
 	struct onenand_chip *this = mtd->priv;
-	struct erase_info instr = {
-		.callback	= NULL,
-	};
+	struct erase_info instr = {};
 	loff_t ofs;
 	int ret;
 	int blocksize = 1 << this->erase_shift;
@@ -219,10 +217,7 @@ static int onenand_block_erase(u32 start, u32 size, int force)
 static int onenand_block_test(u32 start, u32 size)
 {
 	struct onenand_chip *this = mtd->priv;
-	struct erase_info instr = {
-		.callback	= NULL,
-		.priv		= 0,
-	};
+	struct erase_info instr = {};
 
 	int blocks;
 	loff_t ofs;
diff --git a/drivers/mtd/altera_qspi.c b/drivers/mtd/altera_qspi.c
index 7bac599a54..d31391f36a 100644
--- a/drivers/mtd/altera_qspi.c
+++ b/drivers/mtd/altera_qspi.c
@@ -153,7 +153,6 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr)
 				putc('\n');
 			instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
 			instr->state = MTD_ERASE_FAILED;
-			mtd_erase_callback(instr);
 			return -EIO;
 		}
 		flash = pdata->base + addr;
@@ -177,7 +176,6 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr)
 				writel(stat, &regs->isr); /* clear isr */
 				instr->fail_addr = addr;
 				instr->state = MTD_ERASE_FAILED;
-				mtd_erase_callback(instr);
 				return -EIO;
 			}
 			if (flash_verbose)
@@ -189,7 +187,6 @@ static int altera_qspi_erase(struct mtd_info *mtd, struct erase_info *instr)
 		addr += mtd->erasesize;
 	}
 	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
 
 	return 0;
 }
diff --git a/drivers/mtd/cfi_mtd.c b/drivers/mtd/cfi_mtd.c
index 78293caa2f..2295bb7220 100644
--- a/drivers/mtd/cfi_mtd.c
+++ b/drivers/mtd/cfi_mtd.c
@@ -58,7 +58,6 @@ static int cfi_mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 		}
 
 		instr->state = MTD_ERASE_DONE;
-		mtd_erase_callback(instr);
 		return 0;
 	}
 
diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
index 684bc94998..af3c4765c4 100644
--- a/drivers/mtd/mtdconcat.c
+++ b/drivers/mtd/mtdconcat.c
@@ -338,14 +338,6 @@ concat_write_oob(struct mtd_info *mtd, loff_t to, struct mtd_oob_ops *ops)
 	return -EINVAL;
 }
 
-static void concat_erase_callback(struct erase_info *instr)
-{
-	/* Nothing to do here in U-Boot */
-#ifndef __UBOOT__
-	wake_up((wait_queue_head_t *) instr->priv);
-#endif
-}
-
 static int concat_dev_erase(struct mtd_info *mtd, struct erase_info *erase)
 {
 	int err;
@@ -358,7 +350,6 @@ static int concat_dev_erase(struct mtd_info *mtd, struct erase_info *erase)
 	init_waitqueue_head(&waitq);
 
 	erase->mtd = mtd;
-	erase->callback = concat_erase_callback;
 	erase->priv = (unsigned long) &waitq;
 
 	/*
@@ -498,8 +489,6 @@ static int concat_erase(struct mtd_info *mtd, struct erase_info *instr)
 	if (err)
 		return err;
 
-	if (instr->callback)
-		instr->callback(instr);
 	return 0;
 }
 
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 582129d0df..37c5b6386e 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -906,13 +906,6 @@ void __put_mtd_device(struct mtd_info *mtd)
 }
 EXPORT_SYMBOL_GPL(__put_mtd_device);
 
-/*
- * Erase is an asynchronous operation.  Device drivers are supposed
- * to call instr->callback() whenever the operation completes, even
- * if it completes with a failure.
- * Callers are supposed to pass a callback function and wait for it
- * to be called before writing to the block.
- */
 int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
 	if (instr->addr > mtd->size || instr->len > mtd->size - instr->addr)
@@ -922,7 +915,6 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
 	if (!instr->len) {
 		instr->state = MTD_ERASE_DONE;
-		mtd_erase_callback(instr);
 		return 0;
 	}
 	return mtd->_erase(mtd, instr);
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 6ab481a7b1..a435ce6d07 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -456,27 +456,6 @@ static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
 	return ret;
 }
 
-void mtd_erase_callback(struct erase_info *instr)
-{
-	if (!instr->callback)
-		return;
-
-	if (instr->mtd->_erase == part_erase && instr->len) {
-		if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
-			instr->fail_addr -= instr->mtd->offset;
-		instr->addr -= instr->mtd->offset;
-	}
-
-	instr->callback(instr);
-
-	if (instr->mtd->_erase == part_erase && instr->len) {
-		if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
-			instr->fail_addr += instr->mtd->offset;
-		instr->addr += instr->mtd->offset;
-	}
-}
-EXPORT_SYMBOL_GPL(mtd_erase_callback);
-
 static int part_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 {
 	return mtd->parent->_lock(mtd->parent, ofs + mtd->offset, len);
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 3679ee727e..b3dcaf0704 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -3605,10 +3605,6 @@ erase_exit:
 	chip->select_chip(mtd, -1);
 	nand_release_device(mtd);
 
-	/* Do call back function */
-	if (!ret)
-		mtd_erase_callback(instr);
-
 	/* Return more or less happy */
 	return ret;
 }
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index 09daa0dd36..bb93a9a1e9 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -1836,9 +1836,6 @@ int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
 erase_exit:
 
 	ret = instr->state == MTD_ERASE_DONE ? 0 : -EIO;
-	/* Do call back function */
-	if (!ret)
-		mtd_erase_callback(instr);
 
 	/* Deselect and wake up anyone waiting on the device */
 	onenand_release_device(mtd);
diff --git a/drivers/mtd/spi/sf_mtd.c b/drivers/mtd/spi/sf_mtd.c
index 04de868080..0aed28a52b 100644
--- a/drivers/mtd/spi/sf_mtd.c
+++ b/drivers/mtd/spi/sf_mtd.c
@@ -46,7 +46,6 @@ static int spi_flash_mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 	}
 
 	instr->state = MTD_ERASE_DONE;
-	mtd_erase_callback(instr);
 
 	return 0;
 }
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 124594f0cd..822bd63b92 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -916,7 +916,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 	div_u64_rem(instr->len, mtd->erasesize, &rem);
 	if (rem) {
 		ret = -EINVAL;
-		goto erase_err_callback;
+		goto err;
 	}
 
 	addr = instr->addr;
@@ -964,14 +964,13 @@ erase_err:
 	if (!ret)
 		ret = err;
 
-erase_err_callback:
+err:
 	if (ret) {
 		instr->fail_addr = addr_known ? addr : MTD_FAIL_ADDR_UNKNOWN;
 		instr->state = MTD_ERASE_FAILED;
 	} else {
 		instr->state = MTD_ERASE_DONE;
 	}
-	mtd_erase_callback(instr);
 
 	return ret;
 }
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index b8b878b918..14be95b74b 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -304,18 +304,6 @@ int ubi_io_write(struct ubi_device *ubi, const void *buf, int pnum, int offset,
 	return err;
 }
 
-/**
- * erase_callback - MTD erasure call-back.
- * @ei: MTD erase information object.
- *
- * Note, even though MTD erase interface is asynchronous, all the current
- * implementations are synchronous anyway.
- */
-static void erase_callback(struct erase_info *ei)
-{
-	wake_up_interruptible((wait_queue_head_t *)ei->priv);
-}
-
 /**
  * do_sync_erase - synchronously erase a physical eraseblock.
  * @ubi: UBI device description object
@@ -346,7 +334,6 @@ retry:
 	ei.mtd      = ubi->mtd;
 	ei.addr     = (loff_t)pnum * ubi->peb_size;
 	ei.len      = ubi->peb_size;
-	ei.callback = erase_callback;
 	ei.priv     = (unsigned long)&wq;
 
 	err = mtd_erase(ubi->mtd, &ei);
diff --git a/env/onenand.c b/env/onenand.c
index c8da3ff811..1faa2cb62a 100644
--- a/env/onenand.c
+++ b/env/onenand.c
@@ -73,9 +73,7 @@ static int env_onenand_save(void)
 #endif
 	loff_t	env_addr = CONFIG_ENV_ADDR;
 	size_t	retlen;
-	struct erase_info instr = {
-		.callback	= NULL,
-	};
+	struct erase_info instr = {};
 
 	ret = env_export(&env_new);
 	if (ret)
diff --git a/fs/yaffs2/yaffs_mtdif.c b/fs/yaffs2/yaffs_mtdif.c
index d338f9aa91..50fed2d4b1 100644
--- a/fs/yaffs2/yaffs_mtdif.c
+++ b/fs/yaffs2/yaffs_mtdif.c
@@ -145,7 +145,6 @@ int nandmtd_EraseBlockInNAND(struct yaffs_dev *dev, int blockNumber)
 	ei.len = dev->data_bytes_per_chunk * dev->param.chunks_per_block;
 	ei.time = 1000;
 	ei.retries = 2;
-	ei.callback = NULL;
 	ei.priv = (u_long) dev;
 
 	/* Todo finish off the ei if required */
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 3b302fb8c3..7455400981 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -51,7 +51,6 @@ struct erase_info {
 	u_long retries;
 	unsigned dev;
 	unsigned cell;
-	void (*callback) (struct erase_info *self);
 	u_long priv;
 	u_char state;
 	struct erase_info *next;
@@ -535,16 +534,6 @@ extern int unregister_mtd_user (struct mtd_notifier *old);
 #endif
 void *mtd_kmalloc_up_to(const struct mtd_info *mtd, size_t *size);
 
-#ifdef CONFIG_MTD_PARTITIONS
-void mtd_erase_callback(struct erase_info *instr);
-#else
-static inline void mtd_erase_callback(struct erase_info *instr)
-{
-	if (instr->callback)
-		instr->callback(instr);
-}
-#endif
-
 static inline int mtd_is_bitflip(int err) {
 	return err == -EUCLEAN;
 }
diff --git a/include/nand.h b/include/nand.h
index 80dd6469bc..efe193078b 100644
--- a/include/nand.h
+++ b/include/nand.h
@@ -68,7 +68,6 @@ static inline int nand_erase(struct mtd_info *info, loff_t off, size_t size)
 	instr.mtd = info;
 	instr.addr = off;
 	instr.len = size;
-	instr.callback = 0;
 
 	return mtd_erase(info, &instr);
 }
-- 
2.32.0

Re: [PATCH u-boot-spi v2 1/9] mtd: spi-nor-core: Try cleaning up in case writing BAR failed

[Pratyush Yadav]

On 25/09/21 07:33PM, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> Use the cleanup codepath of spi_nor_erase() also in the event of failure
> of writing the BAR register.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH u-boot-spi v2 2/9] mtd: spi-nor-core: Check return value of write_enable() in spi_nor_erase()

[Pratyush Yadav]

On 25/09/21 07:33PM, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> The spi_nor_erase() function does not check return value of the
> write_enable() call. Fix this.

This is the case for many more calls for write_enable(), but we can fix 
them later I suppose.

Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
> Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>  drivers/mtd/spi/spi-nor-core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 529e7e9178..d2018ab702 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -929,7 +929,9 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
>  		if (ret < 0)
>  			goto erase_err;
>  #endif
> -		write_enable(nor);
> +		ret = write_enable(nor);
> +		if (ret < 0)
> +			goto erase_err;
>  
>  		ret = spi_nor_erase_sector(nor, addr);
>  		if (ret < 0)
> -- 
> 2.32.0
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH u-boot-spi v2 5/9] mtd: spi-nor-core: Don't check for zero length in spi_nor_erase()

[Pratyush Yadav]

On 25/09/21 07:33PM, Marek Behún wrote:
> From: Marek Behún <marek.behun@nic.cz>
> 
> This check is already done in mtdcore's mtd_erase(), no reason to do
> this here as well.

But do we always get here via mtd_erase()? What about "sf erase"? I 
looked at the code and I don't see any checks for 0 length there.

> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>  drivers/mtd/spi/spi-nor-core.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 9e936cbe1a..211eea22a4 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -912,9 +912,6 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
>  	dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr,
>  		(long long)instr->len);
>  
> -	if (!instr->len)
> -		return 0;
> -
>  	div_u64_rem(instr->len, mtd->erasesize, &rem);
>  	if (rem)
>  		return -EINVAL;
> -- 
> 2.32.0
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH u-boot-spi v2 5/9] mtd: spi-nor-core: Don't check for zero length in spi_nor_erase()

[Marek Behún]

On Tue, 28 Sep 2021 22:29:11 +0530
Pratyush Yadav <p.yadav@ti.com> wrote:

> On 25/09/21 07:33PM, Marek Behún wrote:
> > From: Marek Behún <marek.behun@nic.cz>
> > 
> > This check is already done in mtdcore's mtd_erase(), no reason to do
> > this here as well.  
> 
> But do we always get here via mtd_erase()? What about "sf erase"? I 
> looked at the code and I don't see any checks for 0 length there.

Hello Pratyush, you are right.

This function is also called from include/spi_flash.h static inline
function spi_flash_erase(), when CONFIG_IS_ENABLED(DM_SPI_FLASH) is
false.

I think I should move this test to the static inline imlpementation,
before calling mtd->_erase(). This should be done in the caller at one
place, not in all _erase() implementations.

Marek

Re: [PATCH u-boot-spi v2 5/9] mtd: spi-nor-core: Don't check for zero length in spi_nor_erase()

[Pratyush Yadav]

On 01/10/21 11:25AM, Marek Behún wrote:
> On Tue, 28 Sep 2021 22:29:11 +0530
> Pratyush Yadav <p.yadav@ti.com> wrote:
> 
> > On 25/09/21 07:33PM, Marek Behún wrote:
> > > From: Marek Behún <marek.behun@nic.cz>
> > > 
> > > This check is already done in mtdcore's mtd_erase(), no reason to do
> > > this here as well.  
> > 
> > But do we always get here via mtd_erase()? What about "sf erase"? I 
> > looked at the code and I don't see any checks for 0 length there.
> 
> Hello Pratyush, you are right.
> 
> This function is also called from include/spi_flash.h static inline
> function spi_flash_erase(), when CONFIG_IS_ENABLED(DM_SPI_FLASH) is
> false.
> 
> I think I should move this test to the static inline imlpementation,
> before calling mtd->_erase(). This should be done in the caller at one
> place, not in all _erase() implementations.

We would do that. Or we could just leave it here. I don't see it doing 
much harm.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.