diff mbox series

[RESEND,u-boot-spi,6/8] mtd: spi-nor-core: Call mtd_erase_callback() from spi_nor_erase()

Message ID 20210714235109.25228-7-marek.behun@nic.cz
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series Fix `mtd erase` when used with mtdpart | expand

Commit Message

Marek Behún July 14, 2021, 11:51 p.m. UTC 
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>
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(-)

Comments

Simon Glass July 20, 2021, 6:32 p.m. UTC | #1 
On Wed, 14 Jul 2021 at 17:51, Marek Behún <marek.behun@nic.cz> wrote:
>
> 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>
> 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(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>
diff mbox series

Patch

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 8ae033c576..927823930c 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;
 }