On 26.10.2022 09:29, Dario Binacchi wrote:
[External email]
Hi Mikhail,
On Mon, Oct 24, 2022 at 1:24 PM Mikhail Kshevetskiy mikhail.kshevetskiy@iopsys.eu wrote:
On 24.10.2022 12:44, Dario Binacchi wrote:
[External email]
From: Mikhail Kshevetskiy mikhail.kshevetskiy@iopsys.eu
As reported by patch [1], the `mtd erase' command should not erase bad blocks. To force bad block erasing you have to use the `mtd erase.dontskipbad' command.
This patch tries to fix the same issue without modifying code taken from the linux kernel, in order to make further upgrades easier.
[1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kerne... Suggested-by: Michael Trimarchi michael@amarulasolutions.com Co-developed-by: Michael Trimarchi michael@amarulasolutions.com Signed-off-by: Michael Trimarchi michael@amarulasolutions.com Co-developed-by: Dario Binacchi dario.binacchi@amarulasolutions.com Signed-off-by: Dario Binacchi dario.binacchi@amarulasolutions.com Tested-by: Mikhail Kshevetskiy mikhail.kshevetskiy@iopsys.eu Signed-off-by: Mikhail Kshevetskiy mikhail.kshevetskiy@iopsys.eu
Changes in v3:
- Simplify the code. mtd_erase() can't return a bad block error. Print the messaged where the bad block is found.
Changes in v2:
- Change the commit author
- Do not continue to erase if scrub option is enabled and a bad block was found but return from the function.
- Update the patch tags.
cmd/mtd.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c index ad5cc9827d55..29b2a9c04c0c 100644 --- a/cmd/mtd.c +++ b/cmd/mtd.c @@ -434,19 +434,27 @@ static int do_mtd_erase(struct cmd_tbl *cmdtp, int flag, int argc, erase_op.mtd = mtd; erase_op.addr = off; erase_op.len = mtd->erasesize;
erase_op.scrub = scrub; while (len) {ret = mtd_erase(mtd, &erase_op);if (ret) {/* Abort if its not a bad block error */if (ret != -EIO)
if (!scrub) {ret = mtd_block_isbad(mtd, erase_op.addr);if (ret < 0) {printf("Failed to get bad block at 0x%08llx\n",erase_op.addr);ret = CMD_RET_FAILURE;goto out_put_mtd;} else if (ret > 0) {printf("Skipping bad block at 0x%08llx\n",erase_op.addr);ret = -EIO; break;
printf("Skipping bad block at 0x%08llx\n",erase_op.addr);
} }ret = mtd_erase(mtd, &erase_op);if (ret)break;mtd_erase() can return -EIO, see drivers/mtd/nand/spi/core.c function spinand_mtd_erase()
If I compare my original patch [1] with yours [2], I see no difference in behavior except for ret checking after calling mtd_erase() which, in my case, was wrong. Do you agree?
yes
Further, checking for a bad block inside the do_mtd_erase(), the mtd_erase() can return -EIO only in the case of a protected block. In case of the scrub option enabled the bad block is erased, otherwise the block is jumped in the do_mtd_erase() without calling the mtd_erase().
I see -EIO error in the following cases
a) physical bad block
b) hardware failures (ex: some timeout expired)
maybe it also returned for protected block as well.
I think that now we can distinguish between a bad block and a protected block inside the do_mtd_erase(), always if it's true that the -EIO error is only returned for bad/protected block by low level operations.
Could you describe your idea in more details. I do not see a way to distinguish bad and protected blocks.
The main problem with your v3 patch is a termination of erasing loop on any mtd_erase() failure. It becomes worse if mtd_erase() returns -EIO. In this case you'll stop erasing due to error, but do_mtd_erase() will return success.
Mikhail
[1] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.... [2] https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork....
Thanks and regards, Dario
len -= mtd->erasesize; erase_op.addr += mtd->erasesize; }-- 2.32.0
--
Dario Binacchi
Embedded Linux Developer
dario.binacchi@amarulasolutions.com
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310 info@amarulasolutions.com
https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.amarula...